bito-code-review[bot] commented on PR #42013: URL: https://github.com/apache/superset/pull/42013#issuecomment-4963378629
<!-- Bito Reply --> The flagged issue is correct. The `applyFiltersIfEnabled()` helper in `DashboardPage.ts` performs a conditional check (`applyButton.isEnabled()`), which can cause the test to skip the click entirely if the button is disabled or missing. This makes the test non-deterministic, as it may pass without actually exercising the "after Apply Filters" path. To resolve this, you should make the action deterministic by asserting the button's state and clicking it directly in the test, or by updating the helper to throw an error if the button is not in the expected state. Given the context, updating the test to explicitly handle the interaction is the recommended approach. ### Proposed Fix In `superset-frontend/playwright/tests/dashboard/delete-display-control.spec.ts`, replace the conditional helper call with an explicit assertion and click: ```typescript // Replace: await dashboardPage.applyFiltersIfEnabled(); const applyBtn = dashboardPage.getApplyFiltersButton(); await expect(applyBtn).toBeEnabled(); await applyBtn.click(); ``` I have checked the available PR comments and there are no other actionable suggestions to address at this time. Would you like me to proceed with any other tasks? **superset-frontend/playwright/tests/dashboard/delete-display-control.spec.ts** ``` const applyBtn = dashboardPage.getApplyFiltersButton(); await expect(applyBtn).toBeEnabled(); await applyBtn.click(); ``` -- 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]
