sadpandajoe commented on code in PR #44945:
URL: https://github.com/apache/superset/pull/44945#discussion_r4400821780
##########
superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/components/RadioButtonControl.test.tsx:
##########
@@ -107,10 +117,10 @@ test('handles multiple clicks correctly', () => {
fireEvent.click(screen.getByText('Option 3'));
fireEvent.click(screen.getByText('Option 1'));
- expect(onChange).toHaveBeenCalledWith('option2');
- expect(onChange).toHaveBeenCalledWith('option3');
- expect(onChange).toHaveBeenCalledWith('option1');
- expect(onChange.mock.calls.length).toBeGreaterThanOrEqual(3);
+ expect(onChange).toHaveBeenCalledTimes(3);
Review Comment:
With `onChange` now coming only from `Radio.Group`, this test can't see
three calls: `setup({ onChange })` never feeds the new value back, so `option1`
stays selected and the third click on 'Option 1' is a re-click that emits
nothing. That leaves two calls and this assertion fails. The same applies to
the tests that still click the already-selected first option and expect a
callback ('handles null values in options' clicking 'None', and the two
'normalizeOption handles ...' tests clicking the only 'Label'). Could these be
run through a small stateful wrapper (or click a non-selected option) so they
exercise real transitions?
--
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]
