codeant-ai-for-open-source[bot] commented on code in PR #38792:
URL: https://github.com/apache/superset/pull/38792#discussion_r2986421738
##########
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx:
##########
@@ -182,20 +184,37 @@ test('triggers onChange when the value changes', async ()
=> {
expect(onChange).toHaveBeenCalled();
});
+
test('triggers onChange when the mode changes', async () => {
const onChange = jest.fn();
render(<CustomFrame onChange={onChange} value={todayNowValue} />, {
store,
});
await waitForElementToBeRemoved(() => screen.queryByLabelText('Loading'));
- userEvent.click(screen.getByTitle('Midnight'));
- expect(await screen.findByTitle('Relative Date/Time')).toBeInTheDocument();
- userEvent.click(screen.getByTitle('Relative Date/Time'));
- userEvent.click(screen.getAllByTitle('Now')[1]);
+
+ const midnightOptions = await screen.findAllByTitle('Midnight');
+ userEvent.click(midnightOptions[0]);
+
+ const relativeOptions = await screen.findAllByTitle('Relative Date/Time');
+ userEvent.click(relativeOptions[0]);
+
+ const nowOptions = await screen.findAllByTitle('Now');
+ userEvent.click(nowOptions[0]);
+
expect(
await screen.findByText('Configure custom time range'),
).toBeInTheDocument();
- userEvent.click(screen.getAllByTitle('Specific Date/Time')[1]);
+
+ await waitFor(async () => {
+ const tabs = screen.getAllByText('Specific Date/Time');
+ if (tabs.length < 2) {
+ const nows = screen.getAllByText('Now');
+ userEvent.click(nows[nows.length - 1]);
+ throw new Error('Waiting for second tab...');
+ }
+ userEvent.click(tabs[1]);
+ }, { timeout: 3000 });
Review Comment:
**Suggestion:** The `waitFor` retry callback performs clicks and throws
intentionally, which means every retry triggers extra UI side effects and can
over-increment `onChange` unpredictably. `waitFor` should only poll for a
condition, then perform the click once after the condition is met. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ DateFilterControl mode-change test becomes flaky in CI.
- ⚠️ Jest call-count assertion intermittently fails.
```
</details>
```suggestion
await waitFor(
() => expect(screen.getAllByText('Specific Date/Time')).toHaveLength(2),
{ timeout: 3000 },
);
const tabs = screen.getAllByText('Specific Date/Time');
await userEvent.click(tabs[1]);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run test `triggers onChange when the mode changes` in
`superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx:188`
with `onChange = jest.fn()`.
2. Execution enters `waitFor` block at `CustomFrame.test.tsx:208-216`; when
`tabs.length <
2`, callback clicks a `Now` option (`line 212`) and deliberately throws
(`line 213`) to
force retry.
3. Each retry repeats the click side effect; in component
`superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx:68-74`,
`onChange(...)` always calls `props.onChange(...)`, incrementing the mock
call count.
4. When retries occur before both tabs appear, extra `onChange` calls are
accumulated,
making final assertion `toHaveBeenCalledTimes(2)` at
`CustomFrame.test.tsx:218`
nondeterministic/flaky.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CustomFrame.test.tsx
**Line:** 208:216
**Comment:**
*Logic Error: The `waitFor` retry callback performs clicks and throws
intentionally, which means every retry triggers extra UI side effects and can
over-increment `onChange` unpredictably. `waitFor` should only poll for a
condition, then perform the click once after the condition is met.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38792&comment_hash=3f4d8328897f14f7e32a30995c33fb55a0e6dcad62529e0188bb99fcb59311c9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38792&comment_hash=3f4d8328897f14f7e32a30995c33fb55a0e6dcad62529e0188bb99fcb59311c9&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]
