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

refactor: [M3-7859]: Upgrade MSW to 2.2.3 #10285

Merged
merged 11 commits into from
Mar 18, 2024

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Mar 14, 2024

Description πŸ“

This update brings MSW 2.2.3 to Cloud Manager

It is a major update with breaking changes, hence the large amount of files touched.

Changes πŸ”„

  • Update MSW version
  • Regenerate file in public dir (needed to bump yargs-parser for that purpose)
  • Update all handlers in serverHandlers.ts (it was not fun)
  • Update all tests

Preview πŸ“·

No visual change to be expected

How to test πŸ§ͺ

Prerequisites

Verification steps

  • turn on MSW and confirm no regression with mock data
  • run test suite and confirm all pass

As an Author I have considered πŸ€”

Check all that apply

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@abailly-akamai abailly-akamai self-assigned this Mar 14, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is all generated via npx msw init

@abailly-akamai abailly-akamai marked this pull request as ready for review March 14, 2024 18:18
@abailly-akamai abailly-akamai requested a review from a team as a code owner March 14, 2024 18:18
@abailly-akamai abailly-akamai requested review from jdamore-linode and mjac0bs and removed request for a team March 14, 2024 18:18
Copy link

github-actions bot commented Mar 14, 2024

Coverage Report: βœ…
Base Coverage: 81.42%
Current Coverage: 81.42%

Copy link
Contributor

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

This is great! Thank you for taking this on πŸ‘ πŸŽ‰

I left two small requests

packages/manager/public/mockServiceWorker.js Outdated Show resolved Hide resolved
);
},
{
once: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

My dev tools are showing a lot of /v4/account/events failures and I'm thinking we could fix that by mocking an empty paginated response after this initial response.

Screenshot 2024-03-14 at 3 45 22β€―PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm i am not seeing this - the once seems to work fine for me in chrome/safari

Copy link
Contributor

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

πŸŽ‰πŸ™πŸΌ

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

πŸš€ Thank you for doing this; I know it was tedious!

Tests passing and the CRUD operations I verified with the MSW mock data looked good.
Screenshot 2024-03-18 at 10 51 07 AM

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Mar 18, 2024
@jdamore-linode
Copy link
Contributor

jdamore-linode commented Mar 18, 2024

(Picture of passing tests)

Surprisingly I'm getting a lot of failures!

Screenshot 2024-03-18 at 2 18 02 PM

The fact that it's passing in CI and on other machines makes me think it's probably a performance issue with my Mac, but @abailly-akamai would you mind if I take a closer look at this / try reproducing / examining any notices and warnings just to rule out any issues before merging? I'll try to follow up before leaving today!

Edit: Never mind! Got only a few failures on re-run, most of them are the same ones that are already present in develop, and the last looks like a fluke. Thanks Alban, nice work!

@abailly-akamai abailly-akamai merged commit e97fbc2 into linode:develop Mar 18, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants