Skip to content

Fix flaky realtime tests#12543

Open
deepshekhardas wants to merge 1 commit into
appwrite:1.9.xfrom
deepshekhardas:fix/12540-flaky-realtime-tests
Open

Fix flaky realtime tests#12543
deepshekhardas wants to merge 1 commit into
appwrite:1.9.xfrom
deepshekhardas:fix/12540-flaky-realtime-tests

Conversation

@deepshekhardas

Copy link
Copy Markdown

What does this PR do?

Fixes flaky E2E realtime tests by addressing two issues:

  1. WebSocket timeouts: Added timeout parameter to all \getWebsocket()\ calls in both \RealtimeConsoleClientTest\ and \RealtimeCustomClientTest\ to prevent tests from hanging indefinitely when the WebSocket connection fails to establish.

  2. Session re-authentication: In \ estChannelAccount, the test performs a password recovery flow which invalidates the current session. Added explicit re-authentication to ensure subsequent WebSocket connections use a valid auth token.

Related issues

Fixes #12540

Adds timeout parameter to getWebsocket() calls to prevent
indefinite hanging. Re-authenticates session after password
recovery in testChannelAccount to ensure valid auth state.
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR addresses two causes of flaky realtime E2E tests: adding explicit timeout parameters to getWebsocket() calls so failing connections don't hang indefinitely, and re-authenticating after the password-recovery flow in testChannelAccount to keep the cached session valid.

  • RealtimeConsoleClientTest.php: Four getWebsocket() calls receive timeout values (10 s / 30 s). Clean and correct.
  • RealtimeCustomClientTest.php: The session re-auth block appended to testChannelAccount is correct. However, the change also partially strips the setup/create section from testChannelDatabaseTransaction, removing the code that defines $databaseId, $collectionId, and $documentId while leaving all assertions and API calls that reference those variables intact — making the test broken.

Confidence Score: 3/5

Safe to merge only after restoring or fully removing the broken testChannelDatabaseTransaction setup; as-is the test will fail with undefined-variable errors.

The timeout additions and re-auth fix are correct, but the partial removal of the transaction-create setup block in testChannelDatabaseTransaction leaves $databaseId, $collectionId, and $documentId undefined throughout the rest of the method and causes the connected-message assertions to test the wrong response shape.

tests/e2e/Services/Realtime/RealtimeCustomClientTest.php — specifically the testChannelDatabaseTransaction method starting at line 3159

Important Files Changed

Filename Overview
tests/e2e/Services/Realtime/RealtimeCustomClientTest.php Adds session re-auth after password recovery (correct fix) and a WebSocket timeout, but partially removes the "Transaction Create" setup block from testChannelDatabaseTransaction while leaving all downstream code that references the now-undefined $databaseId/$collectionId/$documentId variables, breaking that test.
tests/e2e/Services/Realtime/RealtimeConsoleClientTest.php Adds explicit timeout values (10 s and 30 s) to four getWebsocket() calls to prevent indefinite hangs; straightforward and correct.

Comments Outside Diff (1)

  1. tests/e2e/Services/Realtime/RealtimeCustomClientTest.php, line 3177-3179 (link)

    P1 Broken test: undefined variables from removed setup block

    The PR removed the "Setup Database and Collection" and "Test Transaction Create" blocks (which defined $databaseId, $collectionId, and $documentId), but all downstream code that depends on those variables was left in place. Lines 3177–3179 here reference undefined variables, and the same undefined variables are used again in the "Test Transaction Update" (lines 3199–3201, 3221) and "Test Transaction Delete" (lines 3242–3244) sections below. In PHP this produces undefined-variable notices and the string interpolation resolves to empty strings, meaning the assertion on line 3177 tests for an empty event path rather than the intended document path.

    Additionally, $response at this point is the connected message (the second client->receive() that returned the event was removed along with the setup code), so assertArrayHasKey('timestamp', $response['data']) on line 3175 and assertEquals('Transaction Document', $response['data']['payload']['name']) on line 3179 will both fail against a connected-message shape.

    The fix is either to restore the removed setup+create block, or to remove the entire testChannelDatabaseTransaction method rather than leaving it partially intact.

Reviews (1): Last reviewed commit: "fix: resolve flaky realtime tests by add..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant