sadpandajoe commented on code in PR #45051:
URL: https://github.com/apache/superset/pull/45051#discussion_r4252694253
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.test.tsx:
##########
@@ -197,6 +197,15 @@ test('does not fetch data when filter has not been in
view', () => {
expect(mockRequestChartData).not.toHaveBeenCalled();
});
+test('fetches data when inView is false if filter has requiredFirst', () => {
+ const requiredFirstFilter = createMockFilter({
+ requiredFirst: true,
+ });
+ renderFilterValue({ filter: requiredFirstFilter, inView: false });
+
Review Comment:
This test never sets a return value for `mockRequestChartData`, and
`jest.clearAllMocks()` only clears call history. Run alone (`-t 'fetches data
when inView is false if filter has requiredFirst'`), the mock returns
`undefined` and `requestChartDataResolved(...).then` at `FilterValue.tsx:297`
throws inside the effect; in the full file it only passes by inheriting the
rejected promise left by the earlier error-state test. Could you give it an
explicit pending promise, like the loading-spinner test does
(`mockRequestChartData.mockReturnValue(new Promise(() => {}))`)?
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.test.tsx:
##########
@@ -197,6 +197,15 @@ test('does not fetch data when filter has not been in
view', () => {
expect(mockRequestChartData).not.toHaveBeenCalled();
});
+test('fetches data when inView is false if filter has requiredFirst', () => {
Review Comment:
The `filter.filterType !== 'filter_time'` exception in `FilterValue.tsx` has
no test: dropping that clause would still pass this test, and a time filter
with `requiredFirst: true` would then start loading before it is in view. Would
you add a sibling case that renders `requiredFirst: true`, `filterType:
'filter_time'`, `inView: false` and asserts `mockRequestChartData` is not
called?
--
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]
