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: migrate message v8 tests from Python to JS #50421

Merged
merged 2 commits into from
Oct 29, 2023

Conversation

jahjahLemonade
Copy link
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
@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
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@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
56 checks passed
@nodejs-github-bot
Copy link
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
Copy link
Member

targos commented Nov 14, 2023

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 --', '* --');
Copy link
Member

@codebytere codebytere Jan 15, 2024

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

message tests can be migrated from python to JS
8 participants