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.

Sign up for GitHub

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

Jump to bottom

embedtest doesn't actually run any code #49501

Closed
mmomtchev opened this issue Sep 5, 2023 · 1 comment
Closed

embedtest doesn't actually run any code #49501

mmomtchev opened this issue Sep 5, 2023 · 1 comment
Labels
test Issues and PRs related to the tests.

Comments

@mmomtchev
Copy link
Contributor

Version

v21.0.0-pre

Platform

Ubuntu 22.04

Subsystem

No response

What steps will reproduce the bug?

./configure --debug
make -j4
out/Release/embedtest 'require("./test/embedding/test-embedding.js")'
out/Release/embedtest 'require("something")'
out/Release/embedtest 'console.log("something")'
out/Release/embedtest 'throw new Error'

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

Produce output

What do you see instead?

Always a silent success

Additional information

This seems to be very recent because it was working in my mid-July build
@mmomtchev
Copy link
Contributor Author

The argv array is off by one, which subsystem is this?

@mmomtchev mmomtchev mentioned this issue Sep 5, 2023
Closed
@joyeecheung joyeecheung mentioned this issue Sep 5, 2023
Closed
@VoltrexKeyva VoltrexKeyva added the test Issues and PRs related to the tests. label Sep 9, 2023
joyeecheung added a commit that referenced this issue Sep 17, 2023
@joyeecheung
test: fix argument computation in embedtest
76ac548 
There were a few bugs in the original test that went unnoticed
because with the bug the test did not actually get run anymore.
This patch fixes the argument computation by accounting filtering
of the arguments, and uses spawnSyncAndExit{WithoutError} in
the test to enable better logging when the test fails.

PR-URL: #49506
Fixes: #49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
joyeecheung added a commit that referenced this issue Sep 17, 2023
@joyeecheung
build: run embedtest using node executable
1995eca 
We should use the node executable to run this test, instead of
counting on embedtest, the binary being tested, as the test runner.
Otherwise bugs can go unnoticed if the embedtest binary itself
does not work correctly.

PR-URL: #49506
Fixes: #49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 28, 2023
@joyeecheung @ruyadorno
bootstrap: do not expand argv1 for snapshots
6010a91 
To avoid capturing build machine states into the snapshot

PR-URL: #49506
Fixes: #49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 28, 2023
@joyeecheung @ruyadorno
test: fix argument computation in embedtest
1fffda5 
There were a few bugs in the original test that went unnoticed
because with the bug the test did not actually get run anymore.
This patch fixes the argument computation by accounting filtering
of the arguments, and uses spawnSyncAndExit{WithoutError} in
the test to enable better logging when the test fails.

PR-URL: #49506
Fixes: #49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 28, 2023
@joyeecheung @ruyadorno
build: run embedtest using node executable
b30754a 
We should use the node executable to run this test, instead of
counting on embedtest, the binary being tested, as the test runner.
Otherwise bugs can go unnoticed if the embedtest binary itself
does not work correctly.

PR-URL: #49506
Fixes: #49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
@joyeecheung @alexfernandez
bootstrap: do not expand argv1 for snapshots
d4e2c88 
To avoid capturing build machine states into the snapshot

PR-URL: nodejs#49506
Fixes: nodejs#49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
@joyeecheung @alexfernandez
test: fix argument computation in embedtest
375456d 
There were a few bugs in the original test that went unnoticed
because with the bug the test did not actually get run anymore.
This patch fixes the argument computation by accounting filtering
of the arguments, and uses spawnSyncAndExit{WithoutError} in
the test to enable better logging when the test fails.

PR-URL: nodejs#49506
Fixes: nodejs#49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
@joyeecheung @alexfernandez
build: run embedtest using node executable
9b8ad5a 
We should use the node executable to run this test, instead of
counting on embedtest, the binary being tested, as the test runner.
Otherwise bugs can go unnoticed if the embedtest binary itself
does not work correctly.

PR-URL: nodejs#49506
Fixes: nodejs#49501
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
2 participants
@mmomtchev @VoltrexKeyva