Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: refactor snapshots to support multiple files in the same process #53853

Closed
wants to merge 2 commits into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jul 15, 2024

test_runner: add context.filePath

This commit adds a filePath getter to the TestContext and
SuiteContext classes. This allows a context to be mapped back to
the original test file that created it, even if it was imported
from another file. This is useful for mapping features like test
snapshots to the correct test file. This is also prep work for
supporting running test files in the test runner process.

test_runner: refactor snapshots to get file from context

This commit refactors the internals of snapshot tests to get the
name of the test file from the test context instead of passing
it to the SnapshotManager constructor. This is prep work for
supporting running test files in the test runner process.

@cjihrig cjihrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 15, 2024
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jul 15, 2024
doc/api/test.md Outdated Show resolved Hide resolved
doc/api/test.md Outdated Show resolved Hide resolved
@cjihrig cjihrig requested a review from MoLow July 15, 2024 12:46
@cjihrig cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 15, 2024
Copy link
Contributor

Failed to start CI
   ⚠  No approving reviews found
   ℹ  request-ci label was added by a Collaborator after the last push event.
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✔  PR CI job successfully started

[DEBUG] SyntaxError: Unexpected token '<', ")
at Request.json (file:///opt/hostedtoolcache/node/20.15.0/x64/lib/node_modules/@node-core/utils/lib/request.js:53:19)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Request.query (file:///opt/hostedtoolcache/node/20.15.0/x64/lib/node_modules/@node-core/utils/lib/request.js:227:20)
at async Request.gql (file:///opt/hostedtoolcache/node/20.15.0/x64/lib/node_modules/@node-core/utils/lib/request.js:118:22)
at async PRData.getPR (file:///opt/hostedtoolcache/node/20.15.0/x64/lib/node_modules/@node-core/utils/lib/pr_data.js:82:20)
at async RunPRJob.start (file:///opt/hostedtoolcache/node/20.15.0/x64/lib/node_modules/@node-core/utils/lib/ci/run_ci.js:106:7)
at async RunPRJobCommand.start (file:///opt/hostedtoolcache/node/20.15.0/x64/lib/node_modules/@node-core/utils/bin/ncu-ci.js:300:11)
✘ Failed to start CI

https://github.com/nodejs/node/actions/runs/9939801944

@nodejs-github-bot
Copy link
Collaborator

@RedYetiDev RedYetiDev removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Jul 15, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

This commit adds a filePath getter to the TestContext and
SuiteContext classes. This allows a context to be mapped back to
the original test file that created it, even if it was imported
from another file. This is useful for mapping features like test
snapshots to the correct test file. This is also prep work for
supporting running test files in the test runner process.
This commit refactors the internals of snapshot tests to get the
name of the test file from the test context instead of passing
it to the SnapshotManager constructor. This is prep work for
supporting running test files in the test runner process.
@cjihrig
Copy link
Contributor Author

cjihrig commented Jul 15, 2024

Rebased after #53833.

@cjihrig cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 15, 2024

@cjihrig cjihrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Jul 15, 2024
@cjihrig cjihrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 15, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 17, 2024
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jul 17, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/53853
✔  Done loading data for nodejs/node/pull/53853
----------------------------------- PR info ------------------------------------
Title      test_runner: refactor snapshots to support multiple files in the same process (#53853)
Author     Colin Ihrig <cjihrig@gmail.com> (@cjihrig)
Branch     cjihrig:snapshot -> nodejs:main
Labels     semver-minor, author ready, test_runner
Commits    2
 - test_runner: add context.filePath
 - test_runner: refactor snapshots to get file from context
Committers 1
 - cjihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 15 Jul 2024 05:09:30 GMT
   ✔  Approvals: 3
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53853#pullrequestreview-2178415741
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/53853#pullrequestreview-2178504436
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/53853#pullrequestreview-2178740224
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-07-15T20:38:54Z: https://ci.nodejs.org/job/node-test-pull-request/60343/
- Querying data for job/node-test-pull-request/60343/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 53853
From https://github.com/nodejs/node
 * branch                  refs/pull/53853/merge -> FETCH_HEAD
✔  Fetched commits as 0b1ff6965e3b..f4bff7dca35b
--------------------------------------------------------------------------------
[main 60a4b34380] test_runner: add context.filePath
 Author: cjihrig <cjihrig@gmail.com>
 Date: Sun Jul 14 23:37:10 2024 -0400
 4 files changed, 84 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-runner-test-filepath.js
[main 9cfc8e0704] test_runner: refactor snapshots to get file from context
 Author: cjihrig <cjihrig@gmail.com>
 Date: Sun Jul 14 20:33:13 2024 -0400
 5 files changed, 171 insertions(+), 143 deletions(-)
 create mode 100644 test/fixtures/test-runner/snapshots/imported-tests.js
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test_runner: add context.filePath

This commit adds a filePath getter to the TestContext and
SuiteContext classes. This allows a context to be mapped back to
the original test file that created it, even if it was imported
from another file. This is useful for mapping features like test
snapshots to the correct test file. This is also prep work for
supporting running test files in the test runner process.

PR-URL: #53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

[detached HEAD 40a472110e] test_runner: add context.filePath
Author: cjihrig <cjihrig@gmail.com>
Date: Sun Jul 14 23:37:10 2024 -0400
4 files changed, 84 insertions(+), 2 deletions(-)
create mode 100644 test/parallel/test-runner-test-filepath.js
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test_runner: refactor snapshots to get file from context

This commit refactors the internals of snapshot tests to get the
name of the test file from the test context instead of passing
it to the SnapshotManager constructor. This is prep work for
supporting running test files in the test runner process.

PR-URL: #53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

[detached HEAD 62430463a2] test_runner: refactor snapshots to get file from context
Author: cjihrig <cjihrig@gmail.com>
Date: Sun Jul 14 20:33:13 2024 -0400
5 files changed, 171 insertions(+), 143 deletions(-)
create mode 100644 test/fixtures/test-runner/snapshots/imported-tests.js

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/9968639447

@MoLow MoLow added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 17, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 17, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in 0b1ff69...05ca035

nodejs-github-bot pushed a commit that referenced this pull request Jul 17, 2024
This commit adds a filePath getter to the TestContext and
SuiteContext classes. This allows a context to be mapped back to
the original test file that created it, even if it was imported
from another file. This is useful for mapping features like test
snapshots to the correct test file. This is also prep work for
supporting running test files in the test runner process.

PR-URL: #53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Jul 17, 2024
This commit refactors the internals of snapshot tests to get the
name of the test file from the test context instead of passing
it to the SnapshotManager constructor. This is prep work for
supporting running test files in the test runner process.

PR-URL: #53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@cjihrig cjihrig deleted the snapshot branch July 17, 2024 15:35
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
This commit adds a filePath getter to the TestContext and
SuiteContext classes. This allows a context to be mapped back to
the original test file that created it, even if it was imported
from another file. This is useful for mapping features like test
snapshots to the correct test file. This is also prep work for
supporting running test files in the test runner process.

PR-URL: nodejs#53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
This commit refactors the internals of snapshot tests to get the
name of the test file from the test context instead of passing
it to the SnapshotManager constructor. This is prep work for
supporting running test files in the test runner process.

PR-URL: nodejs#53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this pull request Jul 28, 2024
This commit adds a filePath getter to the TestContext and
SuiteContext classes. This allows a context to be mapped back to
the original test file that created it, even if it was imported
from another file. This is useful for mapping features like test
snapshots to the correct test file. This is also prep work for
supporting running test files in the test runner process.

PR-URL: #53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this pull request Jul 28, 2024
This commit refactors the internals of snapshot tests to get the
name of the test file from the test context instead of passing
it to the SnapshotManager constructor. This is prep work for
supporting running test files in the test runner process.

PR-URL: #53853
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
RafaelGSS added a commit that referenced this pull request Jul 30, 2024
Notable changes:

deps:
  * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997
http:
  * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054
inspector:
  * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593
lib,src:
  * drop --experimental-network-imports (Rafael Gonzaga) #53822
meta:
  * add jake to collaborators (jakecastelli) #54004
module:
  * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619
stream:
  * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111
test_runner:
  * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866
  * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853
  * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853

PR-URL: TODO
@RafaelGSS RafaelGSS mentioned this pull request Jul 30, 2024
RafaelGSS added a commit that referenced this pull request Jul 30, 2024
Notable changes:

deps:
  * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997
http:
  * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054
inspector:
  * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593
lib,src:
  * drop --experimental-network-imports (Rafael Gonzaga) #53822
meta:
  * add jake to collaborators (jakecastelli) #54004
module:
  * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619
stream:
  * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111
test_runner:
  * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866
  * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853
  * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853

PR-URL: #54123
RafaelGSS added a commit that referenced this pull request Aug 5, 2024
Notable changes:

deps:
  * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997
http:
  * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054
inspector:
  * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593
lib,src:
  * drop --experimental-network-imports (Rafael Gonzaga) #53822
meta:
  * add jake to collaborators (jakecastelli) #54004
module:
  * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725
stream:
  * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111
test_runner:
  * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866
  * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853
  * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853

PR-URL: #54123
RafaelGSS added a commit that referenced this pull request Aug 5, 2024
Notable changes:

deps:
  * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997
http:
  * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054
inspector:
  * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593
lib,src:
  * drop --experimental-network-imports (Rafael Gonzaga) #53822
meta:
  * add jake to collaborators (jakecastelli) #54004
module:
  * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725
stream:
  * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111
test_runner:
  * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866
  * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853
  * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853

PR-URL: #54123
RafaelGSS added a commit that referenced this pull request Aug 6, 2024
Notable changes:

deps:
  * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997
http:
  * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054
inspector:
  * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593
lib,src:
  * drop --experimental-network-imports (Rafael Gonzaga) #53822
meta:
  * add jake to collaborators (jakecastelli) #54004
module:
  * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725
stream:
  * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111
test_runner:
  * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866
  * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853
  * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853

PR-URL: #54123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. semver-minor PRs that contain new features and should be released in the next minor version. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants