-
Notifications
You must be signed in to change notification settings - Fork 359
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
test: successful login with weak or strong password #9858
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9858 +/- ##
==========================================
- Coverage 54.71% 50.69% -4.02%
==========================================
Files 1261 938 -323
Lines 155988 126646 -29342
Branches 3590 3605 +15
==========================================
- Hits 85342 64204 -21138
+ Misses 70514 62311 -8203
+ Partials 132 131 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Suggestion, but LGTM with or without it.
}); | ||
}); | ||
|
||
it('logs in with weak password', async () => { |
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.
Nit: more descriptive description can make it easier/quicker to diagnose CI failures.
it('logs in with weak password', async () => { | |
it('logs in with warning when using a weak password', async () => { |
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.
LGTM
await waitFor(() => { | ||
// onFinish begins: | ||
expect(screen.getByRole('button')).toBeDisabled(); | ||
}); | ||
await waitFor(() => { | ||
// onFinish ends: | ||
expect(screen.getByRole('button')).not.toBeDisabled(); |
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.
nit: this part isn't in the weak password test, it should probably be in both or neither
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.
there are invalid test cases
Ticket
CM-432
Description
Add unit testing to
DeterminedAuth
to test:Test Plan
No additional testing required, unit test should pass.
Checklist
docs/release-notes/
See Release Note for details.