express icon indicating copy to clipboard operation
express copied to clipboard

New Integration tests (example split continuation)

Open UlisesGascon opened this issue 3 months ago • 2 comments

Main Changes

This PR introduces a new pipeline .github/workflows/integration-tests.yml.

  • Any PR or push will trigger this pipeline, that will execute the .github/workflows/ci.yml in expressjs/examples.
  • We select the branch in scope and the repo, depending if it is a PR from a fork or a PR from the express repo itself.
  • This PR requires a PAT Token to work, see. I am currently using PAT scoped token for UlisesGascon/express-examples

Important

~~While this PRs triggers the process, does not wait until the .github/workflows/ci.yml is completed (fail or success) and also does not provide feedback, for example the action executed URL or similar. I think that this PR is a good step in the right direction but not yet enough in terms of DX~~

Here you can find some screenshots on how it works

From Express fork: Screenshot 2024-02-18 at 18 26 54

Screenshot 2024-02-18 at 18 26 59

From Examples fork:

Screenshot 2024-02-18 at 18 26 06

Screenshot 2024-02-18 at 18 07 03

Context

This PR is supported by https://github.com/UlisesGascon/express-examples/pull/7 and https://github.com/UlisesGascon/express-examples/pull/8

Notes

  • This PR is polluted by #5311 as it is a continuation
  • The pipeline .github/workflows/integration-tests.yml is the only thing in scope in this PR
  • The pipeline .github/workflows/integration-tests.yml will fail until the repo ulisesgascon/express-examples is migrated to expressjs/examples

Changelog

  • 421a7dd feat: add integration tests pipeline by @UlisesGascon
  • 23ad7ed chore: update secret reference in the integration pipeline by @UlisesGascon
  • 6551198 fix: branch reference by @UlisesGascon
  • c377c1c feat: add support for forks in the integration pipeline by @UlisesGascon
  • 2e372c6 chore: update repo reference in the pipeline by @UlisesGascon

UlisesGascon avatar Feb 18 '24 17:02 UlisesGascon

With the changes on https://github.com/expressjs/express/pull/5492/commits/41cecb0c57c8b4372d99d775fbae3082729cd229, I was able to remove dependencies and monitor the execution details for the examples repo (Note that I tested it between my forks).

In the logs the URL of the executed pipeline is included so it is much easier to follow.

Failed case

Screenshot from 2024-04-18 17-19-06 Screenshot from 2024-04-18 17-18-49

Success case

Screenshot from 2024-04-18 17-22-27

Screenshot from 2024-04-18 17-22-36

Other cases

I added a timeout to 10m (harcoded) in case that there are any infra issues but I was not able to simulate that error.

UlisesGascon avatar Apr 18 '24 15:04 UlisesGascon

Some notes that might require us to fine-tune this decision (cc: @expressjs/express-tc and Captains).

As it stands, the idea is to remove the examples from the Express core and keep them as a separate repository at https://github.com/expressjs/examples so they can evolve independently (ES6, latest versions, etc.).

Currently, the examples are used as part of the testing process, so if we separate them, we'll lose the capability to run them locally. Additionally, if we look at the fork (https://github.com/UlisesGascon/express-examples/blob/main/.github/workflows/ci.yml), the idea is to use the newer versions of Node.js, which are more aligned with Express@5 (see: https://github.com/expressjs/express/pull/5595).

So, what do you think?

  • Should we keep the examples in v4 and only use them in v5? Or should we make the effort to have two branches (4.x and 5.x) in the examples repo so we can run the tests against one or the other based on our expectations?
  • Is it okay to ignore the end-to-end tests while developing locally and review the impact of our changes only in the CI? Should we include a script to fetch them, set up the environment, and then allow us to run the tests locally?

UlisesGascon avatar Apr 18 '24 15:04 UlisesGascon