Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Msw 2 #7692

Merged
merged 2 commits into from
Nov 4, 2023
Merged

Msw 2 #7692

merged 2 commits into from
Nov 4, 2023

Conversation

ylavoie
Copy link
Contributor

@ylavoie ylavoie commented Nov 4, 2023

Msw 2 changed the API and required an adjustment of all tests.
It also enforce the validity of URL used in fetches, so we had to make sure that the current location was always set before fetching.

@ylavoie ylavoie requested a review from ehuelsmann November 4, 2023 04:13
Copy link
Member

@ehuelsmann ehuelsmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think this does the job of moving to msw2. I'm not seeing the benefits in our tests for the API changes that msw2 created. However, I guess our tests are too small to show the benefit.

The thing I should have brought to your attention when these tests were originally developed, is that these tests use a different coding style ("modern JavaScript") than the main JS code base: it doesn't use semi-colons to terminate statements. It would have been nice to stick to one style, but I guess that's water under the bridge.

});

it("should fail on bad version", async () => {
const jsdomAlert = window.alert; // remember the jsdom alert
window.alert = jest.fn(); // provide an empty implementation for window.alert
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why empty? Why not capture the fact that it was called (if that's what we expect to happen)?

@ehuelsmann ehuelsmann merged commit 4f8efbe into ledgersmb:master Nov 4, 2023
16 checks passed
@ylavoie ylavoie deleted the msw-2 branch November 7, 2023 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants