rusackas commented on code in PR #39984:
URL: https://github.com/apache/superset/pull/39984#discussion_r3230720187
##########
superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx:
##########
@@ -267,7 +267,7 @@ test('sorts table when clicking column headers', async ()
=> {
.filter(
call =>
call.url.includes('order_column') &&
- call.url.includes('last_saved_at'),
+ call.url.includes('changed_on_delta_humanized'),
Review Comment:
The analysis is plausible but the test actually passes — 19/19 green locally
on the PR branch. The initial page-load fetch doesn't include
\`order_column=changed_on_delta_humanized\` in the URL, presumably because it's
the default sort and react-table / the ListView component doesn't serialize the
default to the query string. So the filter only catches the click-triggered
request, and \`toHaveLength(1)\` holds.
Reproducing:
\`\`\`
npm test -- --testPathPatterns='ChartList.listview' --watchAll=false
…
Test Suites: 1 passed, 1 total
Tests: 19 passed, 19 total
\`\`\`
That said, the underlying concern is reasonable — if anyone later changes
how default sort is serialized, this assertion would silently start matching
the initial load too. Asserting on the latest call (or on sort direction
transition) would be more robust. Not blocking, but worth a follow-up if the
area sees more churn.
--
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]
