← Back to PRs

#18921: fix(mattermost): register websocket listeners before emitting events in test

by BinHPdev open 2026-02-17 05:48 View on GitHub →
channel: mattermost size: XS
## Summary - Problem: The mattermost websocket test "dispatches reaction events to the reaction handler" consistently times out after 120s on CI (both Linux and Windows), blocking **all** CI runs including `main`. - Why it matters: Every open PR fails CI due to this single broken test, preventing any merges. - What changed: Moved event emissions (`emitOpen`, `emitMessage`, `emitClose`) to occur **after** `connectOnce()` is invoked (which synchronously registers listeners in the Promise constructor), instead of before. - What did NOT change (scope boundary): No production code changes. Only the test event ordering was fixed. ## Change Type (select all) - [x] Bug fix - [x] Chore/infra ## Scope (select all touched areas) - [x] Integrations ## Linked Issue/PR - Related: Blocks all 22+ open PRs that currently fail CI ## User-visible / Behavior Changes None ## Security Impact (required) - New permissions/capabilities? No - Secrets/tokens handling changed? No - New/changed network calls? No - Command/tool execution surface changed? No - Data access scope changed? No ## Repro + Verification ### Environment - OS: macOS / Linux CI / Windows CI - Runtime/container: Node 22+ - Integration/channel: Mattermost extension ### Steps 1. Run `pnpm test -- --run extensions/mattermost/src/mattermost/monitor-websocket.test.ts` 2. Before fix: test "dispatches reaction events to the reaction handler" hangs for 120s then fails with timeout 3. After fix: all 3 tests pass in <10ms ### Expected - All mattermost websocket tests pass quickly ### Actual (before fix) - Test times out at 120s because events are emitted before listeners are registered ## Evidence - [x] Failing test/log before + passing after **Before:** `Error: Test timed out in 120000ms.` on every CI run (main and all PRs) **After:** `3 tests passed` in 6ms ### Root cause `createMattermostConnectOnce()` returns an async function that synchronously creates a WebSocket via `webSocketFactory()` and registers event listeners inside a `new Promise()` constructor. The test was calling `socket.emitOpen()`, `socket.emitMessage()`, and `socket.emitClose()` **before** invoking `connectOnce()`, so no listeners were registered yet. Events were silently dropped, the close event never triggered `resolveOnce()`, and the promise hung until the 120s test timeout. Fix: call `const promise = connectOnce()` first (listeners register synchronously), then emit events, then `await promise`. ## Human Verification (required) - Verified scenarios: All 3 mattermost websocket tests pass locally (macOS, Node 22) - Edge cases checked: Confirmed the other two tests already use `queueMicrotask` to defer emissions correctly - What you did **not** verify: CI (will verify via this PR's CI run) ## Compatibility / Migration - Backward compatible? Yes - Config/env changes? No - Migration needed? No ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: Revert the single commit - Files/config to restore: `extensions/mattermost/src/mattermost/monitor-websocket.test.ts` - Known bad symptoms reviewers should watch for: None (test-only change) ## Risks and Mitigations None - test-only change with no production code impact. <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes a test-only timing bug in the Mattermost websocket monitor test suite that was causing CI to hang for 120 seconds and fail. - The "dispatches reaction events to the reaction handler" test was emitting WebSocket events (`emitOpen`, `emitMessage`, `emitClose`) **before** calling `connectOnce()`, which is where event listeners are synchronously registered inside a `Promise` constructor. Events were silently dropped, the promise never resolved, and the test timed out. - The fix calls `const promise = connectOnce()` first so listeners are registered, then emits events, then `await promise`. This matches the pattern already used by the other two tests in the file (which use `queueMicrotask` to defer emissions). - No production code changes. Only the test event ordering was corrected. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it is a minimal, correct test-only fix with no production code changes. - The change is a 3-line test-only fix that corrects an obvious event ordering bug. The fix is consistent with how the other two tests in the same file handle event timing. No production code is modified. - No files require special attention. <sub>Last reviewed commit: f80509a</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs