codeant-ai-for-open-source[bot] commented on code in PR #42013:
URL: https://github.com/apache/superset/pull/42013#discussion_r3574613486
##########
superset-frontend/playwright/tests/dashboard/delete-display-control.spec.ts:
##########
@@ -232,21 +210,13 @@ testWithAssets(
);
// 7. Click Apply Filters.
- const applyBtn = page
- .locator(
- '[data-test="filter-bar__apply-button"],
[data-test="filterbar-action-buttons"] button[type="submit"]',
- )
- .first();
- if (await applyBtn.isEnabled().catch(() => false)) {
- await applyBtn.click();
- }
+ await dashboardPage.applyFiltersIfEnabled();
Review Comment:
**Suggestion:** Using a conditional helper here can silently skip the Apply
action when the button is disabled, so the test may pass without ever
exercising the “after Apply Filters” path it is supposed to validate. Make this
step deterministic by asserting the Apply control is enabled and clicking it
(or failing the test if it is not), so the regression check cannot become a
false positive. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Display Control regression test may pass without Apply click.
- ⚠️ Apply Filters regressions can escape automated detection.
- ⚠️ E2E suite gives false confidence about filter behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Playwright spec
`superset-frontend/playwright/tests/dashboard/delete-display-control.spec.ts`
which
defines the regression test in `testWithAssets(...)` at lines 39-233.
2. Observe that the test seeds a chart and dashboard, removes the "Time
grain" Display
Control via the config modal, then at line 213 calls `await
dashboardPage.applyFiltersIfEnabled();`.
3. In `superset-frontend/playwright/pages/DashboardPage.ts` at lines 134-145,
`applyFiltersIfEnabled()` locates the Apply button and returns early if
`applyButton.isEnabled()` is false or throws, meaning no click occurs when
the control is
disabled or missing.
4. The test then waits for charts to load and asserts the "Time grain"
control count is
zero at lines 214-231; if the control is already gone after saving the
modal, this
assertion passes even though Apply was never clicked, so the supposed "after
Apply
Filters" regression path is not actually exercised.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ff03848c55c948bca5b0a847c56bc413&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=ff03848c55c948bca5b0a847c56bc413&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/playwright/tests/dashboard/delete-display-control.spec.ts
**Line:** 213:213
**Comment:**
*Incomplete Implementation: Using a conditional helper here can
silently skip the Apply action when the button is disabled, so the test may
pass without ever exercising the “after Apply Filters” path it is supposed to
validate. Make this step deterministic by asserting the Apply control is
enabled and clicking it (or failing the test if it is not), so the regression
check cannot become a false positive.
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%2F42013&comment_hash=a166cb3c62aef9ca6832146fd35ebece050643c8b82200d185643a3b3b133312&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42013&comment_hash=a166cb3c62aef9ca6832146fd35ebece050643c8b82200d185643a3b3b133312&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]
