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

Add E2E testing using Cypress #117

Merged
merged 18 commits into from
Oct 7, 2021
Merged

Add E2E testing using Cypress #117

merged 18 commits into from
Oct 7, 2021

Conversation

dinhtungdu
Copy link
Contributor

@dinhtungdu dinhtungdu commented Aug 20, 2021

Description of the Change

This PR:

  • Sets up E2E testing using Cypress.
  • Adds test cases for Podcasts taxonomy and Block editor.
  • Uses GitHub Actions to:
    • Run the test on PRs, develop, and trunk.
    • Test against WordPress's latest development, production, and minimum version.
    • For PRs, the test runs only on the latest stable version for faster execution.
    • When the PR is merged to production, additional core versions are tested.

Alternate Designs

See #116. The reason to choose Cypress over WP E2E testing is explained in #118 (comment)

Applicable Issues

#118

Changelog Entry

Added: E2E testing using Cypress and WordPress Evn.

Copy link

@markjaquith markjaquith left a comment

Choose a reason for hiding this comment

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

Your wp-env GitHub Actions setup here saved me some Googling, so here is some Cypress advice in thanks!

describe('Admin can login and make sure plugin is activated', () => {
it('Can activate plugin if it is deactivated', () => {
cy.login();
cy.visit('http://localhost:8889/wp-admin/plugins.php');

Choose a reason for hiding this comment

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

You can put http://localhost:8889 into cypress.json under baseUrl and then you can just supply a URL starting with the path (or just use your visitAdminPage() Cypress command).

Copy link
Contributor Author

@dinhtungdu dinhtungdu Aug 27, 2021

Choose a reason for hiding this comment

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

@markjaquith Thank you! I refactored this to use visitAdminPage. I put the test domain to cypress.json file before but I removed it in my latest commit. Instead of using hard-coded value, I get it dynamically from wp-env. I do it that way so I can run the test against multiple WP versions. Because you're using wp-env too, it may be useful for you.

Ps: I actually followed your snippet on cypress-io/cypress#909 (comment).

Choose a reason for hiding this comment

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

@dinhtungdu Nice!

I had used a plugin to set the WP username and password if not overridden in cypress.env.json, but I hadn't thought to pull the baseUrl from there.

	if (!config.env.wp_username) {
		config.env.wp_username = 'admin';
		config.env.wp_password = 'password';
	}

Comment on lines 5 to 9
cy.get('body').then(($body) => {
if ($body.find('#deactivate-simple-podcasting').length > 0) {
cy.get('#deactivate-simple-podcasting').click();
}
});

Choose a reason for hiding this comment

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

Doing conditional testing is not recommended in Cypress. It can lead to really brittle tests. For instance, if WordPress started rendering the plugin deactivation buttons with JavaScript, you might check for its presence before it's there. You can end up with weird race conditions where the tests fail on a slow machine, or fail on a fast machine.

Instead of doing conditional testing when you're unsure of the database's state before the test is run, make it so beforehand.

So, before you cy.visit() the plugins page, you could run this:

cy.exec('npm run env run tests-cli wp plugin deactivate simple-podcasting')

And then you you can just do cy.get('#deactivate-simple-podcasting').click();

Choose a reason for hiding this comment

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

cy.exec('npm run env run tests-cli wp plugin deactivate simple-podcasting')

This adds a dependency to the environment (access to the environment). Often it's better to be able to run e2e tests against a given URL without knowing how the environment is actually prepared (docker, local, VM...). This allows running the same suite of tests against multiple environments, comparison... It might not be a requirement here but it's one that can be important sometimes so I just wanted to raise it.

Choose a reason for hiding this comment

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

Yep, that's a good thing to keep in mind. You could also abstract it into cy.deactivatePlugin('simple-podcasting') and handle environment differences there. I'm just wary of conditional code in UI tests, because it means you're going into the test with uncertainty about what you should be encountering, and repeat runs of the test could take different paths and make different assertions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the head-up! This test doesn't need conditional testing actually. The plugin will be activated if the environment is set up correctly. For this case, it's safe to assume that the plugin is activated because the test relies on wp-env.

@helen
Copy link
Contributor

helen commented Aug 25, 2021

Made an issue to track various efforts and the overall conversation in #118

@dinhtungdu dinhtungdu changed the title try: cypress Add E2E testing using Cypress Aug 28, 2021
@dinhtungdu dinhtungdu requested a review from helen August 28, 2021 09:50
@dinhtungdu dinhtungdu self-assigned this Aug 28, 2021
@dinhtungdu dinhtungdu marked this pull request as ready for review August 28, 2021 09:54
@jeffpaul jeffpaul added this to the 1.3.0 milestone Aug 30, 2021
@dinhtungdu dinhtungdu mentioned this pull request Oct 7, 2021
6 tasks
@dinhtungdu
Copy link
Contributor Author

@helen @jeffpaul This PR is ready to review/merge.

@jeffpaul jeffpaul merged commit 2cd7d96 into develop Oct 7, 2021
@jeffpaul jeffpaul deleted the try/cypress branch October 7, 2021 17:09
This was linked to issues Dec 10, 2021
@cadic cadic modified the milestones: 1.3.0, 1.2.1 Dec 16, 2021
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.

Implement E2E / acceptance tests Tests to mock and test feeds
6 participants