-
Notifications
You must be signed in to change notification settings - Fork 896
chore(site): clean up mocks after each test #12805
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
Conversation
@Parkreiner I did some major clean up on |
@code-asher I would appreciate your review on the terminal changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, but I'd like a second opinion from @Parkreiner about the useClipboard
tests :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raised one concern about the new clipboard setup (that might already be resolved – didn't have the chance to play around with the test files)
But if it's no longer an issue, and @code-asher thinks the terminal changes look good, the PR should be good to go
I was not able to get to this today but I will look at it tomorrow. It does seem like at least one terminal test is failing. 🤔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Caught one more area where I think we can simplify one of the useEffect
calls, but aside from that, I think the code is good to merge in as-is
// component. We then observe this attribute for any changes, as we cannot | ||
// rely on other screen elements to indicate completion. | ||
const wrapper = | ||
utils.container.querySelector<HTMLDivElement>("[data-state]")!; | ||
utils.container.querySelector<HTMLDivElement>("[data-status]")!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big fan of the updated name
After making all mocks get fully cleaned up after each test, I realized we have a few fragile tests that may rely on previous mocks or the order of things that are running. This might take some time to "fix" things so I'm going to do this step by step so you can follow the commits.