-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
DEV: Full system specs coverage for signup/login #26977
Conversation
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.
This looks good as is to me (maybe just missing some cleanups). Nice!
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.
Overall looks good to me, left some minor comments. We can merge and fine-tune though.
expect(signup_modal).to be_open | ||
expect(signup_modal).to have_no_password_input | ||
expect(signup_modal).to have_valid_username | ||
expect(signup_modal).to have_valid_email |
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.
Are there any other things to check here? Maybe custom user fields?
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.
@jancernik the new specs here are looking good. Please prioritize merging this before it gets too big.
Merging also helps stress test the specs, they'll run regularly and we will see if there are any flakeys.
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.
Love it 😍
Those specs are a work of art 👨🎨
b9a0094
to
9d3aeb4
Compare
No description provided.