codeant-ai-for-open-source[bot] commented on code in PR #36012:
URL: https://github.com/apache/superset/pull/36012#discussion_r2765145226
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx:
##########
@@ -126,22 +124,38 @@ const datasetResult = (id: number) => ({
show_columns: ['id', 'table_name'],
});
-fetchMock.get('glob:*/api/v1/dataset/1', datasetResult(1));
-fetchMock.get(`glob:*/api/v1/dataset/${id}`, datasetResult(id));
-
-fetchMock.post('glob:*/api/v1/chart/data', {
- result: [
- {
- status: 'success',
- data: [
- { name: 'Aaron', count: 453 },
- { name: 'Abigail', count: 228 },
- { name: 'Adam', count: 454 },
- ],
- applied_filters: [{ column: 'name' }],
- },
- ],
-});
+function setupFetchMocks() {
+ fetchMock.get(`glob:*/api/v1/dataset/${id}`, datasetResult(id));
+ // Mock dataset 1 for buildNativeFilter fixtures which use datasetId: 1
+ fetchMock.get('glob:*/api/v1/dataset/1', datasetResult(1));
Review Comment:
**Suggestion:** The dataset detail fetch mocks in `setupFetchMocks` do not
include a wildcard for query parameters, but `FiltersConfigForm` requests
dataset details using URLs like `/api/v1/dataset/${datasetId}?q=...`, so these
requests won't match `glob:*/api/v1/dataset/${id}` or `glob:*/api/v1/dataset/1`
and will bypass the mock, leading to unexpected real network calls or unhandled
fetches during tests. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Tests may make real network calls causing flakiness.
- ⚠️ Async dataset selects may time out in unit tests.
- ⚠️ CI runs could intermittently fail on these tests.
- ⚠️ Test output shows unmatched fetch warnings.
```
</details>
```suggestion
fetchMock.get(`glob:*/api/v1/dataset/${id}?*`, datasetResult(id));
// Mock dataset 1 for buildNativeFilter fixtures which use datasetId: 1
fetchMock.get('glob:*/api/v1/dataset/1?*', datasetResult(1));
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the test file at
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
and locate setupFetchMocks() defined at lines ~127-158. The test suite calls
setupFetchMocks() in beforeEach() (see beforeEach at ~195).
2. Run the test(s) that open FiltersConfigModal which triggers dataset
detail requests
(the modal's async dataset/column selects are exercised by tests such as
'validates the
default value' and other flows that render the modal). When the component
requests a
dataset detail it issues a URL including query parameters (for example:
/api/v1/dataset/7?q=... or /api/v1/dataset/1?q=...).
3. Because setupFetchMocks registers fetch-mock routes for
`glob:*/api/v1/dataset/${id}`
and `glob:*/api/v1/dataset/1` without a trailing `?*`, requests that include
querystring
parameters do not match those mock routes and therefore bypass the mock. The
real code
will then either hit the network or be handled as an unmocked fetch by
fetch-mock, causing
unexpected behavior or test failures.
4. Observe the failure: tests may perform real network calls, throw errors
about unmatched
fetches, or time out during async selects. Confirm by running the test file
(npm run test
...FiltersConfigModal.test.tsx) and watching for network calls or fetch-mock
unmatched
warnings in the test output.
Note: The beforeEach registration point and the affected tests are concrete
and verifiable
in the file: beforeEach() calling setupFetchMocks() at ~195 and
component-rendering tests
(e.g., 'validates the default value' at ~366) exercise the code path that
triggers dataset
detail fetches.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
**Line:** 128:130
**Comment:**
*Logic Error: The dataset detail fetch mocks in `setupFetchMocks` do
not include a wildcard for query parameters, but `FiltersConfigForm` requests
dataset details using URLs like `/api/v1/dataset/${datasetId}?q=...`, so these
requests won't match `glob:*/api/v1/dataset/${id}` or `glob:*/api/v1/dataset/1`
and will bypass the mock, leading to unexpected real network calls or unhandled
fetches during tests.
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>
--
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]
