E2E testing that requires WordPress instances

[luisherranz]

@mmczaplinski

We need to make sure that the WP instance has the correct permalink structure, because the default docker WP image does not have the “pretty” permalinks which prevents access to the REST API on the /wp-json/ route.

That is a setting on the DB, so you just have to login in WP, change it and dump the database.

[mmczaplinski]

@luisherranz

What about getting rid of the e2e-wp.js file and adding that work in the JS file of each instance (now named start-e2e-docker )? That way we wouldn’t need to add the specs to the workflow, we could add them in that file.

yup, I think that should work I’m trying that approach now.

@luisherranz

• Maybe startWP() and startFrontity() can, not only start docker and frontity, but also await until they are ready using waitOn() .
• Maybe startFrontity can do an npx frontity dev if NODE_ENV is development and npx frontity build && npx frontity serve if it’s production .
• Maybe startCypress can run cypress.open if NODE_ENV is development and cypress.run if it’s production.

I like those

[filipe]

In the spirit of automation and since it seems you will be already using WP-CLI, you can use these commands to change the permalinks without having to login into WP.

wp option get permalink_structure
wp option update permalink_structure '/%postname%'

[mmczaplinski]

Thanks @filipe This is what we’re going to do, indeed.

BTW, we’re working on the E2E testing system in this PR: https://github.com/frontity/frontity/pull/450 (still work in progress) if you’d like to follow the progress

[mmczaplinski]

I’ve made a video summary of the current status of the PoC:

Apologies for the sound quality - I didn’t notice that my headset did not work

[luisherranz]

Thanks for the video Michal. I have some questions:

• Why did you move the WordPress setup from the start/bootstrap routine to the spec files? Wasn’t the whole purpose to do that once and then run all the specs of that instance without having to set it up again?

• Do you have a list of the scripts/tasks that we need? For example:

  • Install plugin.
  • Remove plugin.
  • Replace database.
  • Dump/save database.

• When do you think we’ll be able to start using it?

Regarding the current e2e folder, I would merge it with the new approach within an instance folder that simply doesn’t initilizate WordPress.

[luisherranz]

Maybe if we are going to install/remove plugins and replace the database on each spec file, we are moving away from the need of running several WordPress intances in parallel… One instance would be enough.

I guess it’s going to be slower, but it’ll be simpler.

What do you think @mmczaplinski?

[luisherranz]

We discussed these questions a bit in today’s daily. This is the video:

E2E + WP discussion in Dev Standup

[mmczaplinski]

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.

[luisherranz]

Awesome

I have uploaded our session yesterday:

E2E system - Session 1

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

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

• 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?

[luisherranz]

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.

[mmczaplinski]

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

agreed, makes more sense

@luisherranz

the list of arguments

yup, I think this list of arguments makes sense

@luisherranz

Don’t run Cypress:

Whether to run Cypress or just start the servers.

node e2e.js --no-cypress

[luisherranz]

This was the last session:

E2E system - Session 2

[luisherranz]

I’ve been taking a look at the progress of the PR. Looks great 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
  • browser: chrome
  • wordpress_version: latest
  • target: module (both)
  • publicPath: http://localhost:3001/custom/path

That would be 9 instead of 48

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

[luisherranz]

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

[mmczaplinski]

@luisherranz

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

Yep, this makes sense.

However, the GH Action for cypress requires that

• if you run edge you have to do it using the windows image https://github.com/cypress-io/github-action#edge
• when running firefox, you have to use a different user https://github.com/cypress-io/github-action#firefox

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

[mmczaplinski]

@luisherranz

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

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

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.

[mmczaplinski]

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.

[luisherranz]

@mmczaplinski

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

Ok, I agree

[luisherranz]

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

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!!!