Skip to content
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

Merged
merged 15 commits into from May 17, 2024

Conversation

jancernik
Copy link
Member

No description provided.

Copy link
Contributor

@pmusaraj pmusaraj left a 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!

Copy link
Contributor

@pmusaraj pmusaraj left a 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.

spec/support/omniauth_helpers.rb Show resolved Hide resolved
spec/system/login_spec.rb Outdated Show resolved Hide resolved
spec/system/page_objects/modals/signup.rb Outdated Show resolved Hide resolved
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
Copy link
Contributor

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?

spec/system/social_authentication_spec.rb Show resolved Hide resolved
spec/system/login_spec.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@pmusaraj pmusaraj left a 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.

@jancernik jancernik changed the title WIP: Full system specs coverage for signup/login DEV: Full system specs coverage for signup/login May 17, 2024
@jancernik jancernik marked this pull request as ready for review May 17, 2024 13:52
spec/system/login_spec.rb Outdated Show resolved Hide resolved
Copy link
Member

@ZogStriP ZogStriP left a 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 👨‍🎨

@jancernik jancernik merged commit 57eff8b into discourse:main May 17, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants