Skip to content

test: migrate message v8 tests from Python to JS#50421

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
jahjahLemonade:jl-test-message-v8
Oct 29, 2023
Merged

test: migrate message v8 tests from Python to JS#50421
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
jahjahLemonade:jl-test-message-v8

Conversation

@jahjahLemonade

Copy link
Copy Markdown
Contributor

Migrate the remaining v8 tests in the test/message folder from Python to JS.

Fixes: #47707

Migrated tests:

v8/
	v8_warning.js
	v8_warning.out

Screenshot 2023-10-27 at 12 46 15 AM

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Oct 27, 2023
Comment thread test/parallel/test-node-output-v8-warning.mjs Outdated
@MoLow MoLow added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Oct 28, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2023
@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 29, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 29, 2023
@nodejs-github-bot nodejs-github-bot merged commit 65087c0 into nodejs:main Oct 29, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 65087c0

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#50421
Fixes: nodejs#47707
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
targos pushed a commit that referenced this pull request Nov 11, 2023
PR-URL: #50421
Fixes: #47707
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@targos

targos commented Nov 14, 2023

Copy link
Copy Markdown
Member

This test fails in some specific release conditions. I opened #50724

targos pushed a commit that referenced this pull request Nov 23, 2023
PR-URL: #50421
Fixes: #47707
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@RafaelGSS RafaelGSS mentioned this pull request Nov 28, 2023
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50421
Fixes: #47707
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
.replaceAll('/', '*')
.replaceAll('*test*', '*')
.replaceAll('*fixtures*v8*', '*')
.replaceAll('node --', '* --');

@codebytere codebytere Jan 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MoLow @jahjahLemonade the idea here was to make this generic because Electron (and perhaps some other embedders) run these tests with a different executable name (in our case, of course, Electron). As this stands, it fails for the same reason the * was initially added, unintentionally defeating its purpose. We're hitting this now as we upgrade Node.js versions to 20.11.0, which is why this comment is a little delayed 😅

I'd like to try to find a solution for some of these snapshots that's ideally a bit less brittle, given the previous python approach seemed much more flexible and easier to make more or less specific in its comparisons.

codebytere added a commit to electron/electron that referenced this pull request Jan 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024
* chore: bump node in DEPS to v20.11.0

* module: bootstrap module loaders in shadow realm

nodejs/node#48655

* src: add commit hash shorthand in zlib version

nodejs/node#50158

* v8,tools: expose necessary V8 defines

nodejs/node#50820

* esm: do not call getSource when format is commonjs

nodejs/node#50465

* esm: fallback to readFileSync when source is nullish

nodejs/node#50825

* vm: allow dynamic import with a referrer realm

nodejs/node#50360

* test: skip test-diagnostics-channel-memory-leak.js

nodejs/node#50327

* esm: do not call getSource when format is commonjs

nodejs/node#50465

* lib: fix assert throwing different error messages in ESM and CJS

nodejs/node#50634

* src: fix compatility with upcoming V8 12.1 APIs

nodejs/node#50709

* deps: update base64 to 0.5.1

nodejs/node#50629

* src: avoid silent coercion to signed/unsigned int

nodejs/node#50663

* src: fix compatility with upcoming V8 12.1 APIs

nodejs/node#50709

* chore: fix patch indices

* chore: update patches

* test: disable TLS cipher test

This can't be enabled owing to BoringSSL incompatibilities.

nodejs/node#50186

* fix: check for Buffer and global definition in shadow realm

nodejs/node#51239

* test: disable parallel/test-shadow-realm-custom-loader

Incompatible with our asar logic, resulting in the following failure:

> Failed to CompileAndCall electron script: electron/js2c/asar_bundle

* chore: remove deleted parallel/test-crypto-modp1-error test

* test: make test-node-output-v8-warning generic

nodejs/node#50421

* chore: fixup ModuleWrap patch

* test: match wpt/streams/transferable/transform-stream-members.any.js to upstream

* fix: sandbox is not enabled on arm

* chore: disable v8 sandbox on ia32/arm

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

8 participants