Split E2E Test tasks into multiple subtasks#1811
Split E2E Test tasks into multiple subtasks#1811Mythicaeda wants to merge 13 commits intodevelopfrom
Conversation
Mythicaeda
added
ci
Mythicaeda
force-pushed
the
refactor/speed-up-github-e2eTests
branch
from
ed75cca to
600aa1f
Compare
dandelany
left a comment
dandelany
left a comment
There was a problem hiding this comment.
A few small comments below from reviewing with @Mythicaeda
.github/workflows/test.yml
Outdated
| if: always() | ||
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: ProfileResults |
There was a problem hiding this comment.
discussed with @Mythicaeda & decided to split this into it's own workflow profile-compilation - it's useful for testing/profiling but not to run on every test
.github/workflows/test.yml
Outdated
| uses: actions/cache@v5 | ||
| with: | ||
| path: ./action-server/node_modules | ||
| key: action-${{ runner.os }}-${{ hashFiles('./action-server/package-lock.json') }} |
There was a problem hiding this comment.
Let's update this cache key to also include './action-server/package.json' on the off chance a developer checks in a package.json change but doesn't commit package-lock. This "shouldn't happen" but might cause very confusing results if it did
| with: | ||
| node-version-file: ./action-server/.nvmrc | ||
| cache: npm | ||
| cache-dependency-path: ./action-server/package-lock.json |
There was a problem hiding this comment.
same here re: including package.json as a part of the cache key
There was a problem hiding this comment.
I'm gonna push back on this after reading more on the setup-node action. cache-dependency-path is explicitly looking for lockfiles, it just permits you to hand it multiple lockfiles in the event of a mono-repo like our own.
I get that impression from this section of the action's advanced-usage docs:
If you choose not to use a lockfile, you must ensure that caching is disabled. The cache feature relies on the lockfile to generate a unique key for the cache entry.
.github/workflows/test.yml
Outdated
| # This means Hasura will have time to complete its start-up tasks without needing an extra "sleep 30" command. | ||
| - name: Start Hasura and Postgres | ||
| run: | | ||
| docker compose -f ./e2e-tests/docker-compose-test.yml up -d postgres hasura |
.github/workflows/test.yml
Outdated
| steps: | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v6 | ||
| - name: Cache Sequencing NPM Packages |
There was a problem hiding this comment.
maybe worth looking into: is there a way to save yml steps as variables/snippets at the top & then reference them to avoid having to copy/paste these steps
There was a problem hiding this comment.
I've added several composite actions to split off repeated subtasks
Upon further examination, all Action Server tests are currently unit tests that can be run with the entire system down, not E2E tests
Java currently takes multiple minutes to compile. This is more than sufficient time for Hasura to complete its start up tasks, instead of needing to delay for 30s after starting up.
- using `false` for this test run, but it should be configured to compare to develop (defaults to only caching `main` and `master`, which aren't branches in the repo)
…r" step - Setup Gradle includes the `validate wrapper` step - activate gradle caching during `setup gradle`
2 participants
Description
Currently, the E2E Test task takes 15 minutes to execute, which is absurdly high. This PR aims to improve that number without making the tests flaky.
Breakdown of E2E Suite test speed locally (with services already setup):
:e2eTest:e2eTest:: ~6 mins:dbTest:e2eTest:: ~2 mins:sequencing-server:e2eTest:: ~1 min:action-server:e2eTest:: ~5 secThe first approach is to split the tests by suite into multiple parallel jobs. The suites should already be independent (especially as they're a mixture of Java and TS tests), but by running them in separate jobs we avoid any issues where two suites are trying to alter the DB at the same time. Additionally, by having them as separate jobs, it will be clearer which suite in particular is failing, as opposed to needing to go through the collected folder.
Breakdown of tests after this approach:
unitTests: 6 min (2 min running tests, 3.5 min assembling):e2eTest:e2eTest:: 11 min (5 min running tests, 3.5 min assembling):dbTest:e2eTest:: 4.5 min (4 min running tests):sequencing-server:e2eTest:: 7 min (1.5 min running tests, 3.5 min assembling the Java backend):action-server:e2eTest:: 5 min (10s running tests, 3 min assembling the Java backend)After that, I added the following improvements
e2e-testenvironment, it doesn't actually require the backend to be runningdevelopnode_modulesforaction-serverandsequencing-server. These seemed to not be cached by the gradle caching task, so they're now being manually cachedBreakdown of tests after all of this:
unitTests: 4.5 min (2 min running tests, 2 min assembling):e2eTest:e2eTest:: 9 min (5 min running tests, 2 min assembling):dbTest:e2eTest:: 4.5 min (4 min running tests):sequencing-server:e2eTest:: 5 min (1.5 min running tests, 2 min assembling):action-server:e2eTest:: 30s min (10s running tests)