Test: Comprehensive JS event tracking for lifecycle states#10008
Test: Comprehensive JS event tracking for lifecycle states#10008jellefoks wants to merge 2 commits intoyoutube:26.eapfrom
Conversation
- Injects an event logger array to monitor blur, focus, freeze, resume, and visibilitychange events via DevTools. - Replaces brittle sleep-based timing with deterministic CDP evaluation checks. - Verifies that correct Web API events are dispatched during all Starboard suspend state transitions.
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
There was a problem hiding this comment.
Code Review
This pull request enhances the lifecycle testing tools by introducing a more robust event-driven verification mechanism. Key changes include updating cdp_js_helper.py to support remote hosts and polling for readiness, and modifying test_lifecycle.sh to use a local HTTP server and an injected event logger for precise state transition tracking. Feedback focuses on improving time measurement reliability in Python, addressing potential port conflicts and remote testing limitations in the shell scripts, and refining process termination and string matching logic for better stability.
| start_time = asyncio.get_event_loop().time() | ||
| while (asyncio.get_event_loop().time() - start_time) < total_wait: |
cobalt/tools/test_lifecycle.sh
Outdated
| echo "[TEST] Starting local Python HTTP server..." | ||
| mkdir -p /tmp/cobalt_test_server | ||
| echo "<html><body><h1>Test</h1><input autofocus></body></html>" > /tmp/cobalt_test_server/index.html | ||
| python3 -m http.server 4000 --directory /tmp/cobalt_test_server > /dev/null 2>&1 & |
There was a problem hiding this comment.
The HTTP server is started on a hardcoded port (4000). This can cause conflicts if multiple instances of the test are run on the same machine or if the port is already in use. Additionally, the cleanup logic at the start of the script (lines 25-41) does not check for or terminate stale HTTP server processes from previous failed runs, which could lead to the new server failing to start.
cobalt/tools/test_lifecycle.sh
Outdated
| rm -f $LOG_FILE | ||
|
|
||
| $EXECUTABLE --remote-debugging-port=$PORT --no-sandbox > $LOG_FILE 2>&1 & | ||
| $EXECUTABLE --url="http://127.0.0.1:4000/" --remote-debugging-port=$PORT --no-sandbox > $LOG_FILE 2>&1 & |
There was a problem hiding this comment.
The --url is hardcoded to 127.0.0.1. If the test is run against a remote device (by setting TEST_LIFECYCLE_HOST), Cobalt running on that device will attempt to connect to a server on its own loopback interface rather than the machine running this script. Consider using the actual IP address of the host machine or making this configurable to support remote testing scenarios.
cobalt/tools/test_lifecycle.sh
Outdated
| echo "FAILURE: Cobalt (PID: $COBALT_PID) did not stop after SIGPWR." | ||
| echo "[TEST] Cleaning up Cobalt (PID: $COBALT_PID)..." | ||
| kill -9 $COBALT_PID | ||
| kill -9 $SERVER_PID 2>/dev/null || true |
There was a problem hiding this comment.
Using kill -9 (SIGKILL) for the HTTP server is unnecessary as a first resort. It's better to use SIGTERM (the default for kill) to allow the process to exit gracefully. This applies to both instances where the server is terminated (lines 144 and 148).
| kill -9 $SERVER_PID 2>/dev/null || true | |
| kill $SERVER_PID 2>/dev/null || true |
cobalt/tools/test_lifecycle.sh
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| kill -9 $SERVER_PID 2>/dev/null || true |
There was a problem hiding this comment.
| RESULT=$(vpython3 cobalt/tools/cdp_js_helper.py --port $PORT "$EXPR" 2>/dev/null) | ||
| if [ "$RESULT" == "$EXPECTED" ]; then | ||
| RESULT=$(vpython3 cobalt/tools/cdp_js_helper.py --host $HOST --port $PORT "$EXPR" 2>/dev/null) | ||
| if [[ "$RESULT" == *"$EXPECTED"* ]]; then |
There was a problem hiding this comment.
The transition from an exact match to a substring match ([[ "$RESULT" == *"$EXPECTED"* ]]) is risky and can lead to false positives. For example, if the expected state is visible, this condition would incorrectly pass if the result is an error message containing that word (e.g., ERROR: visible state not reached). Since cdp_js_helper.py now returns clean string representations of the values, an exact match is safer and more reliable.
| if [[ "$RESULT" == *"$EXPECTED"* ]]; then | |
| if [ "$RESULT" == "$EXPECTED" ]; then |
- Injects an event logger array to monitor blur, focus, freeze, resume, and visibilitychange events via DevTools. - Replaces brittle sleep-based timing with deterministic CDP evaluation checks. - Verifies that correct Web API events are dispatched during all Starboard suspend state transitions.
062ce4f to
9542dea
Compare
This PR enhances the
test_lifecycle.shintegration test to actively monitor and verify standard DOM lifecycle events (focus, blur, freeze, and unfreeze/resume) during Starboard OS state transitions.Key Additions & Robustness
window.event_log) via DevTools to actively recordfocus,blur,freeze,resume, andvisibilitychangeevents dispatched by the application. This ensures that the web application receives the proper standard Web APIs signaling when the underlying platform transitions between states.data:text/htmlURL payload using a simple HTML string. This completely bypasses QuotaDatabase IO locks, ServiceWorker database conflicts, and complex JavaScript garbage collection crashes (often seen when testing against heavy remote origins like YouTube TV), ensuring reliable testing of the underlying event delivery without temporary file conflicts.document.hasFocus()anddocument.visibilityStateaccurately reflect the true lifecycle state prior to verifying the individual dispatched events. We're testing more comprehensively and more reliably.Bug: 448156280