-
Notifications
You must be signed in to change notification settings - Fork 23
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
Added integration tests of the password reset process #205
base: master
Are you sure you want to change the base?
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.
Good find on capybara-email. Couple nits and one request. Thanks!
|
||
scenario 'Participant can reset password' do | ||
# Event needed until Pull Request #202 is merged into the code base | ||
Event.create!(name: 'Event 1', date: Time.now) |
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.
Why not extract this to a let
?
click_button 'Reset Password' | ||
assert page.has_text?('No participant was found with email address [email protected]') | ||
|
||
# Enter a valid email address |
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.
I'd make each one of these individual example blocks. it "enters a valid email address" do
etc...
assert page.has_text?('Instructions to reset your password have been emailed to you') | ||
|
||
# Open and follow instructions | ||
open_email(user.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.
Ohhh I didnt know about this... I like it.
assert page.has_css?('h1', text: 'Update your password') | ||
assert page.has_text?('Please enter the new password below') | ||
fill_in('password', with: 'MN Tech Community') | ||
sleep 4.1 |
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.
We'll want to remove this. If you specify a find('.some-selector.or-whatever', wait: 4)
you can specify a default max wait time. You might want to check but it might already be 5 or more seconds.
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.
I believe this is the only substantial change requested. I'd be happy to merge this if we can remove the sleep call.
No description provided.