vitest: fix several tests #666
Conversation
- Updates the password queries for sign in page to select correct elements - updates PersonalProfile tests to correctly select navigation and tab elements - Makes the tests in CivicProfile and PersonalProfile `todo` instead of commenting out entire file The following are fixed: - test/components/CivicProfileForms/FinancialInfo.test.jsx - test/components/Signup/PodRegistrationForm.test.jsx - test/pages/CivicProfile.test.jsx - test/pages/PersonalProfile.test.jsx - test/pages/Signup.test.jsx ~
|
😈😈😈😈😈😈😈😈😈😈😈😈😈😈😈 |
There was a problem hiding this comment.
Tests for PodRegistrationForm and Signup looks good (they passed the tests as intended with npm run test), can approve this for merging.
Although, now that I look at the two unit tests more, it feels like CivicProfile.test.jsx and PersonalProfile.test.jsx are supposed to be testing the same file (seeing that they both are rendering the CivicProfile component), but somehow the names diverged over time. Think we can kill one of the two test files.
The .todo()s are also a good idea until we get hard fixes on those tests. 👍
As for the unit test on Signup.test.jsx, think you and Andy could sort out which unit test update to keep from your two branches.
|
We don't have a component called I can look closer tomorrow. I haven't compared the our different tests for |
There was a problem hiding this comment.
I agree that PersonalProfile.test isn't really doing a whole lot. It seems like that should be testing the Profile page but it doesn't.
There should be tests written for that instead of this. But I'm fine if it's not done with this specific PR, since we're already going to have multiple ones to fully finish the issue.
If not going to convert it into Profile.test.jsx and then make tests for it, I think PersonalProfile.test could be removed. Then we can create those tests later.
As for the Signup tests, we can just use yours. Mine are practically identical.
|
The unit test for PersonalProfile.test.jsx does not test the Profile component, but the CivicProfile component (see what the test is trying to render, there no imports for the Profile component). We should merge the tests in there as part of CivicProfile.test.jsx. |
|
@AJSterner Any updates? I'm working on my own branch and would like to merge |
|
@andycwilliams @leekahung consolidated the CivicProfile tests that were in the |
This PR for #662:
todoinstead of commenting out entire fileResolves the following tests:
Future Steps/PRs Needed to Finish This Work (optional):
I believe this fixes all the tests.