codeant-ai-for-open-source[bot] commented on code in PR #42012:
URL: https://github.com/apache/superset/pull/42012#discussion_r3574313959
##########
superset-frontend/src/middleware/asyncEvent.test.ts:
##########
@@ -173,6 +173,83 @@ describe('asyncEvent middleware', () => {
expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength(1);
});
+ test('backs off exponentially when polling requests keep failing', async
() => {
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
Review Comment:
**Suggestion:** These tests switch to fake timers after the suite-level
`beforeEach` has already called `init` with real timers, which can leave a real
polling timeout running in parallel with the fake clock. That can produce extra
background `/async_event/` calls and flaky assertions on call counts.
Initialize fake timers before any `init` call for these cases (or avoid the
earlier real-timer init path) so all scheduled polls are controlled by the same
clock. [race condition]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Polling backoff tests may flake due to extra timers.
⚠️ Flakiness undermines confidence in async polling backoff behavior.
⚠️ CI runs may intermittently fail on asyncEvent tests.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test suite including `describe('asyncEvent middleware',
...)` in
`superset-frontend/src/middleware/asyncEvent.test.ts:31-374`, focusing on the
`describe('polling transport', ...)` block at lines 104-280.
2. Observe the `beforeEach` for the polling transport at
`asyncEvent.test.ts:112-121`,
which calls `asyncEvent.init(config)` with real timers; in
`superset-frontend/src/middleware/asyncEvent.ts:249-270`, `init()` (when
`isFeatureEnabled` is true as configured in the outer `beforeEach` at lines
91-95) calls
`loadEventsFromApi()` and then schedules a polling timeout via
`window.setTimeout(loadEventsFromApi, getPollingDelay())` at lines 186-201,
storing the
timeout ID in `pollingTimeoutId`.
3. In the test `test('backs off exponentially when polling requests keep
failing', ...)`
at `asyncEvent.test.ts:176-214`, note that `jest.useFakeTimers()` is called
at line 177
after the suite-level `beforeEach` has already scheduled the real polling
timeout, and
only then does the test reinitialize the middleware with
`asyncEvent.init(config)` at line
181 and start a listener via
`asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() =>
{});` at line 182.
4. Because `jest.useFakeTimers()` at line 177 switches Jest to fake timers
after the real
`setTimeout` created in `init()` (from the `beforeEach`) has already been
scheduled, the
original polling timeout may continue to run on the real clock and is not
reliably
controlled by `jest.advanceTimersByTimeAsync(...)` in this test (lines
185-209); this can
cause extra background `/api/v1/async_event/` polls via `fetchEvents` in
`asyncEvent.ts:186-201`, leading to
`fetchMock.callHistory.calls(EVENTS_ENDPOINT)`
returning higher counts than expected at assertions like line 188 (expecting
1 call) and
line 199 (expecting 2 calls), making these tests flaky depending on timing
and Jest's
timer implementation.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fe26a31a31674b4fa72b93a445bc578d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=fe26a31a31674b4fa72b93a445bc578d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/middleware/asyncEvent.test.ts
**Line:** 177:181
**Comment:**
*Race Condition: These tests switch to fake timers after the
suite-level `beforeEach` has already called `init` with real timers, which can
leave a real polling timeout running in parallel with the fake clock. That can
produce extra background `/async_event/` calls and flaky assertions on call
counts. Initialize fake timers before any `init` call for these cases (or avoid
the earlier real-timer init path) so all scheduled polls are controlled by the
same clock.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42012&comment_hash=465d58a7538960c4804041d31f495a8d1cf874c1251be7b473f5cd1b0e967aed&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42012&comment_hash=465d58a7538960c4804041d31f495a8d1cf874c1251be7b473f5cd1b0e967aed&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
