chore: Split unit and e2e tests into separate CI jobs#651
Conversation
jancurn
changed the title
|
Hey @jirimoravcik @bliuchak I consolidated the unit and e2e tests, as well as GitHub Actions, it looked somewhat messy. As a follow up, I'd like to introduce support + tests for Bun, and enable higher versions of Node in tests. |
|
@bliuchak but looking at the epic, i don't see any PRs, just issues. So perhaps this is good to go now? |
|
Not sure why GH don't show these PR 😞 @jancurn here they are:
They need to be merged in the PR train as they depend on each other. Honestly, there is not much work left; I just need to find some time to finish it. |
|
Cheers, fair enough, will wait with this |
Reorganises tests to mirror apify/mcpc: - Move pure unit tests to `test/unit/` (`tools.js`) and the network/server-backed tests to `test/e2e/`. Shared helpers stay in `test/utils/`. SSL fixtures move alongside the e2e tests. - Add `test:unit` and `test:e2e` npm scripts; `npm test` still runs both suites under nyc with the `--insecure-http-parser` Node option that the e2e suite needs. - Split the Check workflow into a `unit` job that runs the matrix (Node.js 20/22/24) on ubuntu-24.04 with `fail-fast: false` plus lint on Node.js 24, and a single `e2e` job that runs on Node.js 24 with the `localhost-test` hosts entry.
jancurn
force-pushed
the
claude/consolidate-github-actions-tests-b6npb
branch
from
bc70210 to
fed0d26
Compare
jancurn
changed the title
jancurn
changed the title
|
@bliuchak I believe this is now ready for review |
|
@jancurn thanks, i'll take a look into it! |
jirimoravcik
left a comment
jirimoravcik
left a comment
There was a problem hiding this comment.
One comment, otherwise looks good
| @@ -33,8 +34,28 @@ jobs: | |||
| if: ${{ matrix.node-version == 24 }} | |||
| run: npm run lint | |||
|
|
|||
| - name: Run unit tests | |||
| run: npm run test:unit | |||
There was a problem hiding this comment.
This job still contains lint, it should either be reflected in the name or separated into another job
There was a problem hiding this comment.
Good call — extracted lint into its own dedicated Lint job alongside Unit tests and E2E tests in 31d794d.
Generated by Claude Code
Per review on #651: the unit job ran lint on Node 24 only, which made the job name misleading and coupled a lint failure to unit-test reporting on that one matrix entry. Lint now runs as a dedicated job alongside unit and e2e.
6 participants
Summary
Rebuilt on top of v3.0 (master). Mirrors the layout used in apify/mcpc: split unit tests from e2e tests so the unit suite is fast and easy to run as a smoke check, while the heavier e2e suite is its own job.
Master already runs the matrix on Node.js 20/22/24 /
ubuntu-24.04, so this PR no longer touches the Node version policy or the Docker harness — just the test layout and the workflow shape.Changes
Test layout (
test/)test/unit/tools.js— utility-function tests (no network/servers)test/e2e/*.js— server-/network-backed integration tests (moved fromtest/*.js); SSL fixtures (ssl.key,ssl.crt) move alongside them, soimport.meta.dirnamekeeps workingtest/utils/— shared helpers (unchanged); e2e files now import from../utils/...'../src/...'→'../../src/...')npm scripts (
package.json)test:unit—mocha 'test/unit/**/*.js'(no--insecure-http-parser, nonyc; the unit suite doesn't need them)test:e2e—nyc cross-env NODE_OPTIONS=--insecure-http-parser mocha 'test/e2e/**/*.js'test— runs both as one mocha invocation, preservingnpm test -- <file>for the Docker entrypointCI workflow (
.github/workflows/check.yaml)unit— matrix on Node.js[20, 22, 24],runs-on: ubuntu-24.04,fail-fast: false. Lint runs once on Node.js 24.e2e— single job, Node.js 24 /ubuntu-24.04, sets thelocalhost-testhosts entry, thennpm run test:e2e.workflow_callretained, sorelease.yamlkeeps working.Docs
test/README.mdupdated for the new layout / commands.What I dropped from the previous attempt
The earlier revision of this PR predated v3.0; v3.0 already absorbed several things I had patched, so they're gone now:
test/Dockerfileandscripts/test-docker-all.shto Node 20/22/24 (master did it)count_target_bytes.tslint fix (master removed thetypeSockethelper entirely in chore: remove typeSocket assertion helper #653)"exit": truein.mocharc.jsonand pinning e2e to Node 18 / ubuntu-22.04 (master's e2e suite already passes on Node 24 / ubuntu-24.04 in ~3 min, so neither workaround is needed)Test plan
npm run lintcleannpm run test:unitpasses locally (4/4)mocha --dry-run test/e2e/**/*.jsdiscovers 2186 teststest/e2e/ee-memory-leak.jsandtest/e2e/http-agent.jspass with the new import pathsunitmatrix green on Node 20/22/24e2ejob green on Node 24https://claude.ai/code/session_01QyJuystdjqYTCFBNW7Z3XD