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

[v20.x backport] test: relocate test files fixtures for better understanding #48787 #49174

Closed
wants to merge 115 commits into from

Conversation

rluvaton
Copy link
Member

Backport: #48787

@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 Issues and PRs related to the tests. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. labels Aug 14, 2023
rluvaton added a commit to rluvaton/node that referenced this pull request Aug 14, 2023
Backport-PR-URL: nodejs#49174
PR-URL: nodejs#48787
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@rluvaton rluvaton force-pushed the backport-48787-to-v20.x branch from 8b98c18 to 6514d06 Compare August 14, 2023 20:15
rluvaton added a commit to rluvaton/node that referenced this pull request Aug 15, 2023
PR-URL: nodejs#48787
Backport-PR-URL: nodejs#49174
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@rluvaton rluvaton force-pushed the backport-48787-to-v20.x branch from 6514d06 to 2dca814 Compare August 15, 2023 09:33
Comment on lines -67 to -70
assert.match(stdout, /ok 2 - this should pass/);
assert.match(stdout, /not ok 3 - this should fail/);
assert.match(stdout, /ok 4 - .+subdir.+subdir_test\.js/);
assert.match(stdout, /ok 5 - this should pass/);
Copy link
Member Author

@rluvaton rluvaton Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this, as this is like that on main and was like that at the time of the original PR.

how was it added anyway?

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

@nodejs-github-bot
Copy link
Collaborator

rluvaton and others added 15 commits August 15, 2023 14:34
Signed-off-by: Michael Dawson <[email protected]>
PR-URL: nodejs#48876
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Co-authored-by: Luigi Pinca <[email protected]>
PR-URL: nodejs#48611
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#48888
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#48860
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#48892
Fixes: nodejs#48887
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#48896
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
PR-URL: nodejs#48898
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Ruy Adorno <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Refs: nodejs/single-executable#73
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs#48191
Fixes: nodejs/single-executable#73
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#48828
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#48828
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: nodejs#48894
PR-URL: nodejs#48903
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
We can now get the binding data through the reference to the
realm directly, so remove it from the context embedder data
slot.

PR-URL: nodejs#48836
Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: nodejs#48907
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs#46662
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
aduh95 and others added 22 commits August 15, 2023 15:47
PR-URL: nodejs#49069
Fixes: nodejs#49026
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
This commit adds the delimiters ^ and $ to the regex that matches the
MIME types for `data:` URLs.

PR-URL: nodejs#49104
Fixes: nodejs#48957
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#49114
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49158
Reviewed-By: Debadree Chatterjee <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
- Build a static table of octal strings and use it instead of
  building octal strings repeatedly during printing.
- Print a newline and an offset for every 64 bytes in the case
  of printing array literals so it's easier to locate
  variation in snapshot blobs.
- Rework the printing routines so that the differences are only
  made in a WriteByteVectorLiteral routine. We can update this
  for compression support in the future.
- Rename Snapshot::Generate() that write the data as C++ source
  instead of a blob as Snaphost::GenerateAsSource() for clarity,
  and move the file stream operations into it to streamline
  error handling.

PR-URL: nodejs#48851
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
This commit reverts the revert in
bb52656. It also includes the
fix for the issue that required the revert
(nodejs#49059 (comment))
and an additional common.mustCall() in the added test.

Refs: nodejs#49059
Refs: nodejs#49110
PR-URL: nodejs#49116
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49119
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#49033
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Replace `type` with `Type: {Promise}`. Plus, fix incorrect
verb(`creates` -> `returns`) in description.

PR-URL: nodejs#49121
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#49124
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#49034
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: nodejs#49129
Refs: nodejs#49120
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#49129
Refs: nodejs#49120
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
- Always check stderr before stdout as the former would contain error
  information.
- Always match the full stdout to avoid surprises.
- Use `deepStrictEqual` when appropriate to get more informative test
  failures.
- Remove leading slashes from relative paths/URLs to not confuse them
  with absolute paths.
- Remove unnecessary `--no-warnings` flag.

PR-URL: nodejs#49131
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
PR-URL: nodejs#49146
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49125
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#49126
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Subsystems: blob, child_process, common, crypto, http, http2,
readline, repl, snapshot, trace_events

PR-URL: nodejs#49127
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#49128
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49143
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@rluvaton rluvaton mentioned this pull request Aug 15, 2023
PR-URL: nodejs#48787
Backport-PR-URL: nodejs#49174
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@rluvaton rluvaton force-pushed the backport-48787-to-v20.x branch from 2dca814 to 7c07077 Compare August 15, 2023 20:39
rluvaton added a commit to rluvaton/node that referenced this pull request Aug 15, 2023
PR-URL: nodejs#48787
Backport-PR-URL: nodejs#49174
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@RafaelGSS RafaelGSS force-pushed the v20.x-staging branch 2 times, most recently from 2a9aa46 to 21949c4 Compare August 17, 2023 11:48
@rluvaton rluvaton closed this Aug 17, 2023
@rluvaton rluvaton deleted the backport-48787-to-v20.x branch August 17, 2023 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.