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

Update UBE tests #80

Merged
merged 3 commits into from
Jun 2, 2021
Merged

Update UBE tests #80

merged 3 commits into from
Jun 2, 2021

Conversation

jhnstn
Copy link
Member

@jhnstn jhnstn commented Jun 2, 2021

I noticed that the UBE test steps for a connected Jetpack were unclear. This step was incorrect:

Tap the (?) icon and expect to see an option to enable Login with WordPress.com.

Instead I saw the prompt to "Open Jetpack Security settings" .
I updated the pre steps to first allow logging in with wp.com to 1. simplify the test steps and 2. avoid any future changes to
the Jetpack security auth flows.

I also added a checklist template to the top of the doc

2. Connect the Jetpack plugin (it comes pre-installed) to a WP.com account
3. By default, Gutenberg is the default editor so no action required there
2. Connect Jetpack to the signed in WP.com account
3. Visit https://wordpress.com/settings/security, choose newly created site and enable "WordPress.com log in"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this change because it keeps this test focused on Jetpack-connected self-hosted sites. Do you think a separate test should be added at some point to test the "enable Jetpack WP.com SSO flow"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the same. I'm not sure where the that test should live; in the Jetpack domain or Gutenberg. Regardless it's a good idea to have test coverage around that flow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I think it might be fine for it to live here, if you agree I'll add one here.
I made an issue to do that here: #81

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one non-blocking comment, rest looks great! Thank you!

@jhnstn jhnstn force-pushed the update/UBE-test branch from 1ea5e12 to ce06c8e Compare June 2, 2021 21:24
@jhnstn jhnstn merged commit 5827147 into trunk Jun 2, 2021
@jhnstn jhnstn deleted the update/UBE-test branch June 2, 2021 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants