Task: update e2e GitHub workflow file to use Playwright runner

[alexkuc]

Update file e2e-tests.yml to use Playwright runner instead of Playwright in library mode

References

• GitHub Actions
• CI GitHub Actions

[alexkuc]

@tn3rb I'll need your input here. We discussed 2 modes of operation - testing using docker and testing on MENW staging. I believe, it would be best to separate it into 2 separate processes (e2e codebase can be the same though with slight variations to support this).

We discussed the following E2E flow:

 work branch -> PR -> unit tests -> dev branch -> E2E tests -> master branch

However, I am thinking now, do we really need a dev branch? Why not run E2E as a part of PR check? We initially assumed that E2E will take several hours however:

• if E2E fails, we shouldn't merge PR but go back and fix it because that PR will introduce regression
• we don't know yet for how long the E2E will run, it is just an assumption/ballpark figure of several hours

[alexkuc]

Don't forget to build assets and make sure barista plugin is activated too!

[tn3rb]

We discussed 2 modes of operation - testing using docker and testing on MENW staging.

ok so to be clear, the platform used for testing is inconsequential to me. As far as i'm concerned that is a technical decision best left to whoever is engineering the testing system (ie: YOU! 😆). The final decision should be whatever works best based on whatever criteria ends up being most important, which i'm not even sure of atm. SO everything is subject to change... until it's not! 😆

I believe, it would be best to separate it into 2 separate processes

sure, imho, this is the kind of technical decision best left to the engineer(s) doing the work as opposed to the person managing said engineer(s).

Why not run E2E as a part of PR check?

my only concern is the length of time they will take to run. Even the current PHP unit tests take too long to run imho (most of the PHP unit tests are technically integration tests due to the way that the WordPress testing framework is set up) and i may be breaking these up in the future so that only "pure" unit tests run during PR pushes. Longer running integration tests would then run later on in the CI/CD pipeline similar to what i am envisioning for e2e tests.

...if E2E fails, we shouldn't merge PR

ya this is a really important point... like HUGELY important (is "hugely" a word? 🤔) and will most certainly effect the final configuration of our CI/CD pipeline. What might be the best compromise would be to move integration and e2e tests, as well as PR merging, into a workflow that is manually triggered via a PR comment. So after all code reviews and modifications are completed, the PR author would enter a comment like: /merge or merge-me or whatever, and that would trigger a workflow to run which would do the following:

• run PHP integration tests (currently long running "unit" tests)
• run e2e tests
• if the above pass, then merge the PR into DEV

i think i can set up branch protection rules that would restrict PR merging to only the EE Bot user account, who would be responsible for performing the merge. This would satisfy your concerns (and mine) with having PRs that fail tests progressing along the CI/CD pipeline further than they should be allowed.

[alexkuc]

I think your last comment perfectly covers everything for now. I'll proceed with the implementation as you have mentioned.

[alexkuc]

For environment provisioning it's either DDEV or Lando. My next step would be to create a simple criteria of what we need, attempt to implement it with both tools and see which one is more suitable. For design consideration, take into account comments from #1234.

[alexkuc]

Evaluation criteria

• mount folder that would contain result of git pull
• ability to extend base image, e.g. to install PlayWright dependencies
• ability to combine PHP & Node stacks without much hassle
• ability to change PHP & Node version(s) without much hassle
• ability to scaffold multiple projects, e.g. to support Playwright parallelism

[alexkuc]

DDEV

• to mount a local directory, override docker-compose file
• a number of options exist to extend base image
• how to change PHP version and how to change Node version
• there is no ready-made recipe for multiple projects but it looks doable and seems more implementation detail, e.g. create multiple projects or configure multiple database services

[alexkuc]

Lando

• there are default mounts as well as ability to override them
• base image can be overridden
• how to change PHP version and how to change Node version
• parallelism could be achieved either via multiple projects or via multiple databases

[alexkuc]

Setting up repository, e.g. downloading dependencies, installing them, compiling assets, etc. should not be part of MakeEnv.ts otherwise that will lead to tight coupling of the code and possible fragility of the implementation.

Edit: In other words, DDEV scaffolder should not care about environment, it should be ready. It should just execute the necessary DDEV commands to achieve the desired result in a repeatable and scalable fashion.

[alexkuc]

Requires dependencies:

• Docker
• mkcert
• DDEV
• PlayWright w/ browsers

[alexkuc]

In this (barista) repository, unit tests are run only partially as a part of CI checks. To trigger all unit tests, we would need to include test-all in the commit message:

https://github.com/eventespresso/barista/blob/9beb83fd3934ded848db11f934bf1eff9a972de2/.github/workflows/pr-checks.yml#L50-L55

Perhaps, a similar strategy could be employed for E2E where during PR checks only a subset of E2E suites will be run. And once PR is approved, the entire suite of E2E tests would run.

[alexkuc]

Perhaps, for e2e that's not doable to due to Jest tests (unit tests) being tightly coupled to actual source code where code would be able to infer --changedSince for appropriate files include source. Whereas E2E will be running against https://<website> and programmatic inferral would be way more difficult.

[alexkuc]

My idea was to use tags, however, it will require a programmatic way to determine which tags to run during CI checks

[alexkuc]

I will proceed to implement this as it seems like the best trade-off between time, complexity and performance:

What might be the best compromise would be to move integration and e2e tests, as well as PR merging, into a workflow that is manually triggered via a PR comment. So after all code reviews and modifications are completed, the PR author would enter a comment like: /merge or merge-me or whatever, and that would trigger a workflow...

[alexkuc]

i think i can set up branch protection rules that would restrict PR merging to only the EE Bot user account, who would be responsible for performing the merge.

@tn3rb this looks totally doable using branch protection rules (requires either GitHub Team or GitHub Free organization)

[alexkuc]

Docker-related caching is currently problematic so I will keep it as it is (i.e. no cache) for the time being. Out-of-the-box, GitHub Actions do not cache official Docker images. Using workaround based on docker save and docker load leads to substantially longer times:

Type	Time
cache restore	~4min
docker pull	~2min

Keep in mind that the task for saving result of docker save to cache took nearly ~10min.

[alexkuc]

In terms of caching installation of tools like mkcert or DDEV, the general consensus is to use a pre-baked Docker image that already includes those installed pre-installed and ready-for-use out-of-the-box. Unfortunately, in our case, it leads to Docker-in-Docker scenario, since DDEV scaffolds WordPress environments using Docker as well. The other possibility is to use a self-hosted runner where these tools would be pre-installed. Currently, running a single test from environment.spec.ts:6 leads to results between 3 and 5 minutes. If we reduce "startup" time (time taken to prepare environment) to an approximate 1 minute, this improvement would look negligible if the overall time to run end-to-end tests would be close to 1 hour. If we consider the current time to prepare environment to be approximately 4 minutes, and reducing it to approximately 1 minute, we would save 3 minute or 5% if the overall time to run the end-to-end tests would be close to 1 hour.

References:

• https://github.com/microsoft/playwright/issues/7249#issuecomment-865244492
• https://github.com/microsoft/playwright/issues/23388#issuecomment-1569930053
• https://playwright.dev/docs/browsers#managing-browser-binaries
• https://stackoverflow.com/a/59277514/4343719

[alexkuc]

Useful links for GitHub workflow examples

• issue_comment on issues only or pull requests only
• Running a workflow when a pull request is approved
• Trigger a github workflow if it matches a particular comment in the Pull Request #25389

[alexkuc]

How to get branch name on event issue_comment: https://github.com/actions/checkout/issues/331

[alexkuc]

Fixed by #1254