Repository navigation
Enable more integration tests on TV emulator - #1188
seungsoo47 wants to merge 2 commits into
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
https://github.com/flutter-tizen/plugins/actions/runs/39234382931/job/118527916097 |
5612fc1 to
4cf3614
Compare
| tizen_rpc_port: ["tv-9.0"] | ||
| tizen_window_manager: ["tv-9.0"] | ||
| url_launcher: ["tv-9.0"] | ||
| video_player: ["tv-9.0"] |
There was a problem hiding this comment.
It is notied that video_player does not officially support the TV emulator.
(However, it looks to work to some extent on the latest binaries.)
Will it be possible to maintain this test going forward?
+) Additionally, both tizen_log and permission_handler are also explicitly noted as not supporting TV.
|
https://github.com/flutter-tizen/plugins/actions/runs/41978765725/job/127083713006 integration_test timed out after waiting for 6 hours. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d81d07ef74
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await completer.future.timeout( | ||
| timeout, | ||
| onTimeout: () { | ||
| timedOut = true; | ||
| process.kill(io.ProcessSignal.sigkill); |
There was a problem hiding this comment.
When the hang is in a spawned child that still holds the pipes open—the exact case the preceding comment handles—process.kill terminates only the immediate flutter-tizen process, while Future.timeout does not cancel either the stdout subscription or the stderr subscription created above. The surviving child and listeners remain live while tools/lib/src/integration_test_command.dart immediately retries at lines 229-232, so the retry can overlap the old test and leave processes/file descriptors on the self-hosted runner; kill the process tree and cancel its pipe subscriptions before returning.
Useful? React with 👍 / 👎.
| // Drain stderr so a full pipe cannot block the process. | ||
| process.stderr.listen(io.stderr.add); |
There was a problem hiding this comment.
When running tools/test/device_test.dart, every runIntegrationTest case uses MockProcess, but its setup stubs only stdout and exitCode at lines 40-42. Mocktail therefore throws on this newly accessed non-nullable stderr getter before any log assertion runs; after that is stubbed, the timeout case also needs kill stubbed to return a bool. Add the corresponding stream and method stubs so the existing device test suite can exercise this change.
Useful? React with 👍 / 👎.
bace272 to
d81d07e
Compare
…lutter Temporary commit for investigating CI hangs on the TV emulator. - tools: the integration test timeout only set a flag and kept waiting for stdout to close, so a hung test blocked the job forever. Kill the process and stop waiting on timeout, and drain stderr. - recipe: disable google_maps_flutter again (test runner not responding).
d81d07e to
6063668
Compare
Enable integration tests on the TV 9.0 emulator for firebase_core, google_maps_flutter, google_sign_in, permission_handler, tizen_log, tizen_notification, video_player and webview_flutter_lwe, and drop the outdated comments that excluded them.
webview_flutter_lwe's test file has an async main, which flutter-tizen test can load since flutter-tizen/flutter-tizen#842.
Testing: ran flutter-tizen test --no-dds integration_test on a TV 9.0 emulator with flutter-tizen main (cd5ad06). google_maps_flutter passed 10/10 and webview_flutter_lwe 5/5. The other six passed 5/5 in earlier runs.