E2E testing that requires WordPress instances

🗣 Feature Discussions Released
24

We have decided that we will simplify the implementation a bit and only run one instance of WP at a time and run the specs in parallel. We will only use the following parameters to paralellize the test runs:

  • WP version
  • build type of the frontity project es / all
  • type of the browser to test in: chrome / FF / electron

Tasks that are remaining for the completion of the e2e tests (this is a non-exhaustive list, as there are probably more smaller tasks that are required):

  • Move the current e2e tests inside the “new” e2e tests. Basically merge the two systems into one.
  • To start the frontity server in dev when in development and build for production when on CI
  • Add an option to either open the cypress test runner or just run the tests when in development
  • Create only one docker-compose file at the root level. We can use .env files to substitute the variables in the docker-compose file thus we do not need to create separate files for each version of wordpress.
  • Move the code that does the per instance setup into a root start.js file.
  • Use the minimist library in the root start.js file to create a “CLI” for launching the e2e tests
  • The database files (db.sql) that are dumped/loaded for particular tests should be stored next to the spec files and not stored per instance as earlier.
  • We can can simplify the tasks for install-plugin, etc. because they will not need to use the WP-instance-specific docker-compose.yml files anymore.
25

Awesome :slight_smile:

I have uploaded our session yesterday:

I’ve tested your latest code on E2E Tests that create custom "test" WordPress instances by michalczaplinski · Pull Request #450 · frontity/frontity · GitHub and it works great! I think we’re really close to finish this :tada::smile:

Some afterthoughts:

1. We can rename the start.js file to e2e.js

So people can do this to run the tests:

cd e2e && node e2e.js

2. This could be the list of arguments

I’d make the defaults more appropiate for running this manually in our local enviornments, so node e2e.js with no args is usually what you need.

Let me know what you think :slight_smile:

  • WordPress version:

    node e2e.js --wp-version 5.4

    Default: latest

  • Target:

    The target option of npx frontity dev/build.

    node e2e.js --target es5

    Default: all

  • Broswer:

    The browser option of Cypress.

    node e2e.js --browser firefox

    Default: chrome

  • Frontity Development/Production:

    Whether to run Frontity in development (npx frontity dev) or production (npx frontity build && npx frontity serve).

    node e2e.js --prod

    Default: It runs Frontity in development.

  • Cypress Open/Run:

    Whether to run Cypress in open mode (npx cypress open) or run (npx cypress run).

    node e2e.js --run

    Default: It runs Cypress in open mode.

Question:

I see you have not included Cyress run/open inside the latest start.js file. Is it better to run it outside, doing node e2e.js && cypress run? Is that to be able to use the Cypress GH Action?

26

I’ve run a couple of tests and it seems like locally it’s going to be easier to run Cypress inside the e2e.js file programatically to pass the WORDPRESS_VERSION env variable, which is needed for the InstallPlugin task.

I’ve pushed a commit with the test. It brings back what you were doing previously:

// Run Cypress.
await cypress.open({ env: { WORDPRESS_VERSION: "latest" } });

As I think it’d be nice to keep using the official Cypress GH action, maybe we can add an additional argument for that case:

  • Don’t run Cypress:

    Whether to run Cypress or just start the servers.

    node e2e.js --no-cypress

    Default: It runs Cypress.

Adding the env variables in the GH action is trivial.

27

agreed, makes more sense :slight_smile:

yup, I think this list of arguments makes sense :+1:

:+1:

28

This was the last session:

29

I’ve been taking a look at the progress of the PR. Looks great :clap: I think only the final details are missing.

These are some of the things we may still be missing:

1. We are going to start too many containers.

Right now we have all these variables in the matrix: https://github.com/frontity/frontity/blob/42c46b74f05729c8c99f85f6f094e2c291cfdbef/.github/workflows/e2e-tests.yml#L10-L14

matrix:
  target: ["both", "module", "es5"]
  wordpress_version: ["latest", "5.4"]
  browsers: ["chrome", "firefox", "edge"]
  publicPath: ["/custom/path", "http://localhost:3001/custom/path/"]

If we add another WordPress version and Firefox, we would spin up 24 containers. If we want to add Edge, we would spin up 48 containers. I don’t think that makes sense. We need to be able to join some of these args.

Let’s see what makes and doesn’t make sense:

  • target:

    • It doesn’t make sense to test both and module, we can test only both.
    • It makes sense to run all the tests using es5 instead of module in each browser because I guess browsers may be different in the way they execute JavaScript.
    • I don’t think it makes sense to test es5 against different versions of WordPress. They are not directly related.
    • I don’t think it makes sense to test es5 against different public paths. They are not directly related.

  • wordpress_version:

    • It doesn’t make sense to test against different targets and public paths anything but the latest version of WordPress. There should not be differences.
    • It may make sense to test against different browsers, I guess. I’m not 100% sure.

  • browser:

    • It doesn’t make sense to test the different browsers against different public paths. There should not be differences.

With those constraints, maybe we could run:

  • Test Chrome
    • browser: chrome
    • wordpress_version: latest
    • target: module (both)
    • publicPath: –
  • Test Firefox
    • browser: firefox
    • wordpress_version: latest
    • target: module (both)
    • publicPath: –
  • Test Edge
    • browser: edge
    • wordpress_version: latest
    • target: module (both)
    • publicPath: –
  • Test ES5 in Chrome
    • browser: chrome
    • wordpress_version: latest
    • target: es5
    • publicPath: –
  • Test ES5 in Firefox
    • browser: firefox
    • wordpress_version: latest
    • target: es5
    • publicPath: –
  • Test ES5 in Edge
    • browser: edge
    • wordpress_version: latest
    • target: es5
    • publicPath: –
  • Test previous WordPress version
    • browser: chrome
    • wordpress_version: 5.4
    • target: module (both)
    • publicPath: –
  • Test custom relative publicPath
    • browser: chrome
    • wordpress_version: latest
    • target: module (both)
    • publicPath: /custom/path
  • Test custom absolute publicPath

That would be 9 instead of 48 :sweat_smile:

We could even reduce it if we need to by mixing some of those together or add more if we think we need to test with another configuration.

The matrix would be the args required for each test:

matrix:
  args:
    - "--wp-version latest --browser chrome --target both"
    - "--wp-version latest --browser firefox --target both"
    - "--wp-version latest --browser edge --target both"
    - "--wp-version latest --browser chrome --target es5"
    - "--wp-version latest --browser firefox --target es5"
    - "--wp-version latest --browser edge --target es5"
    - "--wp-version 5.4 --browser chrome --target both"
    - "--wp-version latest --browser chrome --target both --public-path /custom/path"
    - "--wp-version latest --browser chrome --target both --public-path http://localhost:3001/custom/path"

Apart from that, I made a mistake when we added the --publicPath arg to our CLI. It should have been --public-path because the rest of the CLI args are not using camelcase.

I’ll do a PR to fix it: Change publicPath

30

My bad, we cannot do the matrix that way because we need access to the different variables isolated.

GH actions have another way to define this using include. It is documented here: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-including-additional-values-into-combinations

This would be the proper matrix:

matrix:
  name:
    - Test Chrome
    - Test Firefox
    - Test Edge
    - Test ES5 in Chrome
    - Test ES5 in Firefox
    - Test ES5 in Edge
    - Test previous WordPress version
    - Test custom relative public-path
    - Test custom absolute public-path

  include:
    - name: Test Chrome
      wp-version: latest
      browser: chrome
      target: both

    - name: Test Firefox
      wp-version: latest
      browser: firefox
      target: both

    - name: Test Edge
      wp-version: latest
      browser: edge
      target: both

    - name: Test ES5 in Chrome
      wp-version: latest
      browser: chrome
      target: es5

    - name: Test ES5 in Firefox
      wp-version: latest
      browser: firefox
      target: es5

    - name: Test ES5 in Edge
      wp-version: latest
      browser: edge
      target: es5

    - name: Test previous WordPress version
      wp-version: 5.4
      browser: chrome
      target: both

    - name: Test custom relative public-path
      wp-version: latest
      browser: chrome
      target: both
      public-path: /custom/path

    - name: Test custom absolute public-path
      wp-version: latest
      browser: chrome
      target: both
      public-path: http://localhost:3001/custom/path
1 Like
31

Yep, this makes sense.

However, the GH Action for cypress requires that

So we will have to also add os and user to the matrix.

32

Unfortunately, the GH actions don’t work with this this syntax. :slightly_frowning_face:

In this example, the actions would run one time for each name (with all the other matrix parameters set to "") and then run once for each object included in the include section.

I’ve tried a slightly different approach:

e2e-tests:
    runs-on: ${{ matrix.os }}

    strategy:
      matrix:
        name: [Test chrome]
        wp-version: [latest]
        browser: [chrome]
        target: [both]
        image: ["cypress/browsers:node12.18.0-chrome83-ff77"]
        os: [ubuntu-16.04]
        options: ["--user 1000"]

        include:
          - name: Test Firefox
            wp-version: latest
            browser: firefox
            target: both
            image: cypress/browsers:node12.18.0-chrome83-ff77
            os: ubuntu-16.04
            options: --user 1001

          - name: Test Edge
            wp-version: latest
            browser: edge
            target: both
            image: cypress/browsers:node12.18.0-chrome83-ff77 # <--- This is the problem
            os: ubuntu-16.04
            options: --user 1001
            
          ## etc...

The first issue is that we have to specify each matrix variable explicitly when using the include, which is a bit verbose.

Secondly, and more importantly, because we have to specify a different image for running firefox we will also have to specify the image when running edge. However, the tests for edge should not be running in a custom container!

I think the best solution for now is to make a chrome-e2e workflow, a firefox-e2e workflow and edge-e2e workflow.

Unfortunately, this will mean that there is some repetition across those workflows.

However, I think that in the future we could use the Composite Run Steps feature that github has just released to compose the “initial” steps and then create 3 separate jobs
for chrome edge and firefox inside of the same workflow file.

33

It seems that getting the tests to work across firefox and edge is a bit harder than just adding some config to the yaml:

If we specify the following as is required to run the tests on firefox:

 container:
      image: cypress/browsers:node12.16.1-chrome80-ff73
      options: --user 1001

Then all the steps are going to run on that container instead of on ubuntu. This image does not have docker-compose installed so we would have to create our own image that includes docker-compose. Here’s a list of all the software installed on the github actions images: https://docs.github.com/en/actions/reference/software-installed-on-github-hosted-runners

The windows tests fail with another problem: https://stackoverflow.com/questions/48066994/docker-no-matching-manifest-for-windows-amd64-in-the-manifest-list-entries

For the moment I will remove the edge and firefox tests and we can include them later on as I think they are not strictly necessary for what we are trying to achieve with the E2E tests.

34

Ok, I agree :slight_smile:

35

I’ve merged the PR: https://github.com/frontity/frontity/pull/450 :clap::clap::clap:

All the information on how to use it is in the README.md file of the e2e folder: https://github.com/frontity/frontity/blob/dev/e2e/README.md

Thanks @mmczaplinski for your work!!! :smile:

4 Likes