codeant-ai-for-open-source[bot] commented on code in PR #42157:
URL: https://github.com/apache/superset/pull/42157#discussion_r3642173348
##########
superset-frontend/src/components/Chart/chartActions.test.ts:
##########
@@ -463,6 +463,113 @@ describe('chart actions', () => {
expect(addWarningToastSpy).not.toHaveBeenCalled();
addWarningToastSpy.mockRestore();
});
+
+ // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
+ describe('GlobalAsyncQueries error handling', () => {
+ beforeEach(() => {
+ (
+ global as unknown as { featureFlags: Record<string, boolean> }
+ ).featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: true,
+ };
+ });
Review Comment:
**Suggestion:** This test block mutates the global feature-flag state but
never restores it, so later tests can run with async queries unintentionally
enabled and become order-dependent/flaky. Save the previous
`global.featureFlags` value and restore it in `afterEach` (or reset to the
suite default) to avoid cross-test state leakage. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ GlobalAsyncQueries flag leaks into later chart tests.
- ⚠️ Order-dependent behavior between async and legacy API tests.
- ⚠️ Potentially flaky tests when files run in different orders.
- ⚠️ Future tests may unknowingly inherit wrong feature flags.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset-frontend/src/components/Chart/chartActions.test.ts` and
locate the
`describe('GlobalAsyncQueries error handling', ...)` block starting at line
468, whose
`beforeEach` at lines 469–475 assigns `global.featureFlags = {
[FeatureFlag.GlobalAsyncQueries]: true }` without saving or restoring the
previous value.
2. Run the Jest test suite so that this file is executed; before each test
in the
`GlobalAsyncQueries error handling` block, the `beforeEach` mutates the
shared
`global.featureFlags` object and leaves it in that state after the last test
in this
`describe` completes.
3. Observe that the subsequent `describe('legacy API', ...)` block starting
at line 576 in
the same file runs without any setup that resets `global.featureFlags`, so
any test in
this block that reads `global.featureFlags` (directly or via application
helpers) will see
`GlobalAsyncQueries` forced to `true`, regardless of the suite’s intended
default.
4. Because Jest executes tests in file order and `global.featureFlags` is a
shared
singleton, the behavior of tests in later `describe` blocks (in this file or
any other
file executed after it) becomes order-dependent and can change if this
`describe('GlobalAsyncQueries error handling')` block is reordered, skipped,
or its
`beforeEach` is modified, demonstrating the cross-test state leakage caused
by not
restoring `global.featureFlags`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2c65a6e604104a2aa65d4bbc270c14ba&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=2c65a6e604104a2aa65d4bbc270c14ba&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/components/Chart/chartActions.test.ts
**Line:** 469:475
**Comment:**
*Possible Bug: This test block mutates the global feature-flag state
but never restores it, so later tests can run with async queries
unintentionally enabled and become order-dependent/flaky. Save the previous
`global.featureFlags` value and restore it in `afterEach` (or reset to the
suite default) to avoid cross-test state leakage.
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%2F42157&comment_hash=f3f6821db399065bd209ab4ffb8b06e4f4eff2300054f28c7e0edc34f2ba5b92&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42157&comment_hash=f3f6821db399065bd209ab4ffb8b06e4f4eff2300054f28c7e0edc34f2ba5b92&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]
