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

Fix and modernize E2E tests #6747

Merged
merged 14 commits into from
Nov 30, 2021
Merged

Fix and modernize E2E tests #6747

merged 14 commits into from
Nov 30, 2021

Conversation

delawski
Copy link
Collaborator

@delawski delawski commented Nov 25, 2021

Summary

Recently, the E2E tests started to fail even though no changes were done to the plugin codebase. The reason is that one of the plugins used in the tests, Autoptimize, has been updated by its authors. The new plugin version introduces a "goodbye survey" opened up in a modal whenever the plugin is deactivated. The E2E test specs were not prepared for such a scenario and started to fail.

I expected an issue like this may occur in the future but there was never a good opportunity and time to address it. Since we are now facing a problem, I decided to update and modernize the E2E test infrastructure.

Use local demo plugin

First of all, the third-party plugin (Autoptimize) has been replaced with a local demo plugin called "E2E Tests Demo Plugin" in 1ef787a. The plugin is very simple – all it does is output some invalid AMP markup that can be picked up by the site scanner.

What's the most important, the plugin is installed from the local filesystem. Thanks to that, the tests are no longer dependant on external sources (well, almost – we're still using Hestia theme in some other tests; this should be addressed in the future, too).

Install necessary plugins and themes upfront

Instead of installing necessary plugins and themes in each test suite, they are now installed once in the bootstrap.js file. Thanks to that, a test suite can activate a plugin or a theme when needed. This should reduce the time needed to run E2E tests. Check out 27b4019.

Improve local and CI test execution

So far, there was a single main script for running E2E tests: npm run test:e2e. It used a common jest.config.js file that extended a config file provided by wp-scripts. The script defined WP_BASE_URL env variable directly on the command line, inside the package.json file (using cross-env package). The CI version of the script, test:e2e:ci, was based on the same test:e2e script.

This approach was problematic. Both local and CI scripts shared the same jest.config.js file. On CI a test progress reporting is expected to be minimal. Because of that, locally we also got just a series of dots instead of useful debugging information. What's more, if a developer used a different domain for a local WordPress instance, a temporary override had to be done to the package.json file (so that the WP_BASE_URL would match the actual WordPress URL).

With 5ab3539, the test:e2e:ci script has been unbound from test:e2e. They now use two separate config files, however the CI script is extending the common jest.config.js file.

The console reporting when running test:e2e is now verbose and helps in local debugging:

Screenshot 2021-11-25 at 19 36 04

Also, thanks to the env-cmd package, developers are now free to define a local .env file where the local WordPress instance URL (and not only that) can be defined (like WP_BASE_URL=http://amp.local).

Ensure React state updates are not performed on unmounted components

After enabling full Jest reporting and running E2E tests locally, I noticed some React console warnings. This has resulted in a minor and only non-infrastructural code change in this PR: 9251c28.

Other minor updates

While working on various parts of the E2E tooling and specs, I went ahead and introduced a few more minor updates:

  • Refactor "Other Settings" E2E tests 86c4d1b
  • Remove unnecessary cleanup functions in E2E tests 2ee236f
  • Introduce general saveSettings util function for E2E tests a443e10
  • Use more robust internal activate/deactivate functions in E2E tests 5d5d846
  • Ensure selector is available before scrolling element into view 1ad41a2

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@delawski delawski added Infrastructure Changes impacting testing infrastructure or build tooling Testing Issues related with Unit, E2E, Smoke, and other testing requirements/needs labels Nov 25, 2021
@delawski delawski added this to the v2.2 milestone Nov 25, 2021
@delawski delawski requested a review from pierlon as a code owner November 25, 2021 19:17
@delawski delawski self-assigned this Nov 25, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2021

Plugin builds for 2c796f8 are ready 🛎️!

@codecov
Copy link

codecov bot commented Nov 25, 2021

Codecov Report

Merging #6747 (2034359) into develop (5405daa) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

❗ Current head 2034359 differs from pull request most recent head ada9aa7. Consider uploading reports for the commit ada9aa7 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #6747      +/-   ##
=============================================
- Coverage      77.00%   76.98%   -0.03%     
  Complexity      6548     6548              
=============================================
  Files            264      264              
  Lines          20901    20907       +6     
=============================================
  Hits           16095    16095              
- Misses          4806     4812       +6     
Flag Coverage Δ
javascript 62.87% <0.00%> (-0.33%) ⬇️
php 77.80% <ø> (ø)
unit 77.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...src/components/site-scan-context-provider/index.js 33.09% <0.00%> (-1.50%) ⬇️

Instead, plugins should be installed by E2E tests "manually", i.e. using the WP admin interface.
@westonruter
Copy link
Member

This PR should suprecede my PR #6751.

…ests

* 'develop' of github.com:ampproject/amp-wp:
  Use CF7 instead of Autoptimize for AMP-incompatible plugin in E2E tests
  Replace autoptimize with akismet for E2E tests
  Pin NodeJS to v14 to match WP core and Gutenberg
  Regenerate package-lock.json at lockfileVersion 1
  Update Gutenberg package dependencies
  Update Gutenberg package dependencies
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the thorough description and the investment in improving E2E.

@westonruter westonruter merged commit 64a0ae2 into develop Nov 30, 2021
@westonruter westonruter deleted the fix/e2e-tests branch November 30, 2021 20:18
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Infrastructure Changes impacting testing infrastructure or build tooling Testing Issues related with Unit, E2E, Smoke, and other testing requirements/needs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants