test: improve E2E test robustness and prevent orphaned emulators#94
test: improve E2E test robustness and prevent orphaned emulators#94
Conversation
- Converted eagerly evaluated `late` client variables to nullables in `e2e_test.dart` to prevent `LateInitializationError` from hijacking `tearDownAll` if setup fails. - Wrapped client disconnections in a `try/finally` block to guarantee the Firebase emulator is signaled to shut down regardless of client errors. - Added a `try/catch` block around `_waitForReady` checking during emulator startup to explicitly kill the underlying process if a timeout occurs.
There was a problem hiding this comment.
Code Review
This pull request updates the E2E tests by replacing late variables with nullable types and adding error handling to the emulator startup and teardown processes. A try/finally block was introduced to ensure the emulator is stopped during cleanup. The review feedback suggests refining the teardown logic to call each client's close method independently, ensuring that a failure in one does not prevent others from being closed.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a TestClientBase abstract class to standardize HTTP client lifecycle management across E2E helper classes and refactors the main test suite to use nullable types for clients and the emulator. Feedback suggests improving code readability in setUpAll by using local non-nullable variables instead of repeated null assertions. Additionally, it is recommended to replace the fixed 2-second delay during emulator startup with a more deterministic readiness check to avoid potential test flakiness.
lateclient variables to nullables ine2e_test.dartto preventLateInitializationErrorfrom hijackingtearDownAllif setup fails.try/finallyblock to guarantee the Firebase emulator is signaled to shut down regardless of client errors.try/catchblock around_waitForReadychecking during emulator startup to explicitly kill the underlying process if a timeout occurs.