Home Original page

test: fix argument computation in embedtest by joyeecheung · Pull Request #49506 · nodejs/node

@nodejs-github-bot added build

Issues and PRs related to build files or the CI.

needs-ci

PRs that need a full CI run.

labels

Sep 5, 2023 
To avoid capturing build machine states into the snapshot
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.
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.

This was referenced

Sep 9, 2023

targos

@joyeecheung

This was referenced

Sep 14, 2023

RaisinTen

RaisinTen

joyeecheung added a commit that referenced this pull request

Sep 17, 2023 
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>

joyeecheung added a commit that referenced this pull request

Sep 17, 2023 
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 pull request

Sep 17, 2023 
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 pull request

Sep 28, 2023 
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 pull request

Sep 28, 2023 
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 pull request

Sep 28, 2023 
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>

This was referenced

Sep 28, 2023

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request

Nov 1, 2023 
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 pull request

Nov 1, 2023 
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 pull request

Nov 1, 2023 
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>