test: add check for root user #19850

danbev · 2018-04-06T10:30:15Z

Currently this test fails if run as the root user:

Mismatched innerFn function calls. Expected exactly 62, actual 42.

The motivation for this is that when building node in a docker
container it is convenient to be able to build as root.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

danbev · 2018-04-06T10:30:56Z

CI: https://ci.nodejs.org/job/node-test-pull-request/14093/

danbev · 2018-04-06T13:13:45Z

node-test-commit-arm-fanned failure looks unrelated

console output:

(forced update)
07:05:03 
07:05:03 real	1m39.285s
07:05:03 user	0m16.580s
07:05:03 sys	0m3.340s
07:05:03 + awk '{print $2}'
07:05:03 + ps -ef
07:05:03 + grep '\[node\] <defunct>'
07:05:03 + xargs -rl kill
07:05:05 + rm -f ****
07:05:05 + git checkout -f refs/remotes/jenkins_tmp
07:09:08 Warning: you are leaving 1 commit behind, not connected to
07:09:08 any of your branches:
07:09:08 
07:09:08   3e90b3866c added binaries
07:09:08 
07:09:08 If you want to keep it by creating a new branch, this may be a good time
07:09:08 to do so with:
07:09:08 
07:09:08  git branch <new-branch-name> 3e90b3866c
07:09:08 
07:09:08 HEAD is now at fe18508e9d... added binaries
07:09:08 
07:09:08 real	4m3.176s
07:09:08 user	0m15.870s
07:09:08 sys	0m46.950s
07:09:08 + git reset --hard
07:10:41 HEAD is now at fe18508e9d added binaries
07:10:41 
07:10:41 real	1m0.089s
07:10:41 user	0m4.700s
07:10:41 sys	0m17.600s
07:10:41 + git clean -fdx
07:10:41 warning: failed to remove out/Release
07:10:41 Removing config.gypi
07:10:41 Removing icu_config.gypi
07:10:41 Removing node
07:10:41 Removing node-ci-exec
07:10:41 Removing out/Release/openssl-cli
07:10:41 Removing out/Release/node
07:10:41 Removing test.tap
07:10:41 Removing test/abort/testcfg.pyc
07:10:41 Removing test/addons-napi/testcfg.pyc
07:10:41 Removing test/addons/testcfg.pyc
07:10:41 Removing test/async-hooks/testcfg.pyc
07:10:41 Removing test/doctool/testcfg.pyc
07:10:41 Removing test/es-module/testcfg.pyc
07:10:41 Removing test/gc/testcfg.pyc
07:10:41 Removing test/internet/testcfg.pyc
07:10:41 Removing test/known_issues/testcfg.pyc
07:10:41 Removing test/message/testcfg.pyc
07:10:41 Removing test/parallel/testcfg.pyc
07:10:41 Removing test/pseudo-tty/testcfg.pyc
07:10:41 Removing test/pummel/testcfg.pyc
07:10:41 Removing test/sequential/testcfg.pyc
07:10:41 Removing test/testpy/__init__.pyc
07:10:41 Removing test/tick-processor/testcfg.pyc
07:10:41 Removing test/timers/testcfg.pyc
07:10:41 Removing tools/test.pyc
07:10:41 Removing tools/utils.pyc
07:10:41 
07:10:41 real	0m14.902s
07:10:41 user	0m0.780s
07:10:41 sys	0m4.510s
07:10:42 Build step 'Execute shell' marked build as failure
07:10:43 TAP Reports Processing: START
07:10:43 Looking for TAP results report in workspace using pattern: *.tap
07:10:43 Did not find any matching files. Setting build result to FAILURE.
07:10:43 Checking ^not ok
07:10:44 Jenkins Text Finder: File set '*.tap' is empty
07:10:44 Notifying upstream projects of job completion
07:10:44 Finished: FAILURE

node-test-commit-windows-faned failure looks unrelated

console output:

c:\workspace\node-test-binary-windows>TASKKILL /F /IM node.exe /T   || TRUE
ERROR: The process "node.exe" not found.

c:\workspace\node-test-binary-windows>TASKKILL /F /IM cctest.exe /T   || TRUE
ERROR: The process "cctest.exe" not found.

c:\workspace\node-test-binary-windows>TASKKILL /F /IM run-tests.exe /T   || TRUE
ERROR: The process "run-tests.exe" not found.

c:\workspace\node-test-binary-windows>TASKKILL /F /IM yes.exe /T   || TRUE
ERROR: The process "yes.exe" not found.

c:\workspace\node-test-binary-windows>rm -rfv test/tmp*   || true

c:\workspace\node-test-binary-windows>if not exist test.tap (
echo "Missing test.tap file"  
 set "EXIT_RETURN_VALUE=1" 
) 

c:\workspace\node-test-binary-windows>if not exist cctest.tap (
echo "Missing cctest.tap file"  
 set "EXIT_RETURN_VALUE=1" 
) 

c:\workspace\node-test-binary-windows>rm -vf ../test.tap   || true

c:\workspace\node-test-binary-windows>rm -vf ../cctest.tap   || true

c:\workspace\node-test-binary-windows>mv test.tap ../test.tap   || true

c:\workspace\node-test-binary-windows>mv cctest.tap ../cctest.tap   || true

Trott · 2018-04-06T22:31:32Z

I've thoroughly confused myself. I would have thought #19554 would have fixed this?

danbev · 2018-04-07T04:41:06Z

I've thoroughly confused myself. I would have thought #19554 would have fixed this?

Unless I'm missing something in that PR, the value of 62 is passed into expectsError when not on windows. The decrement of invalidArgTypeErrorCount does not affect the exact value of the context in mustCallChecks:

{exact: 62, actual: 42, stack: "Error↵    at _mustCallInner (/Users/danielbevenius…e.runMain (internal/modules/cjs/loader.js:719:10)", name: "innerFn"}actual: 42exact: 62name: "innerFn"stack: "Error↵    at _mustCallInner (/Users/danielbevenius/work/nodejs/node/test/common/index.js:456:13)↵    at Object.exports.mustCall (/Users/danielbevenius/work/nodejs/node/test/common/index.js:427:10)↵    at Object.expectsError (/Users/danielbevenius/work/nodejs/node/test/common/index.js:739:18)↵    at Object.<anonymous> (/Users/danielbevenius/work/nodejs/node/test/parallel/test-child-process-spawnsync-validation-errors.js:15:12)↵    at Module._compile (internal/modules/cjs/loader.js:675:14)↵    at Object.Module._extensions..js (internal/modules/cjs/loader.js:689:10)↵    at Module.load (internal/modules/cjs/loader.js:589:32)↵    at tryModuleLoad (internal/modules/cjs/loader.js:528:12)↵    at Function.Module._load (internal/modules/cjs/loader.js:520:3)↵    at Function.Module.runMain (internal/modules/cjs/loader.js:719:10)"__proto__: Object

So I think one solution is to do what this PR does and do the check first and then set the expected value.

Trott · 2018-04-07T04:46:08Z

Unless I'm missing something in that PR, the value of 62 is passed into expectsError when not on windows. The decrement of invalidArgTypeErrorCount does not affect the exact value of the context in mustCallChecks

🤦‍♂️

Got it, thanks.

BridgeAR

LGTM

BridgeAR · 2018-04-08T17:21:15Z

test/parallel/test-child-process-spawnsync-validation-errors.js


 let invalidArgTypeError;
 let invalidArgTypeErrorCount = 62;

-if (common.isWindows) {
+if (common.isWindows || rootUser) {
  invalidArgTypeError =
    common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }, 42);


Suggestion: change this whole part to:

let invalidArgTypeErrorCount = 62; if (common.isWindows || rootUser) invalidArgTypeErrorCount = 42; const invalidArgTypeError = common.expectsError({ ... }, invalidArgTypeErrorCount);

That looks good. I'll also remove the decrementing of the count. Thanks

Currently this test fails if run as the root user: Mismatched innerFn function calls. Expected exactly 62, actual 42. The motivation for this is that when building node in a docker container it is convenient to be able to build as root.

danbev · 2018-04-10T05:23:31Z

CI: https://ci.nodejs.org/job/node-test-pull-request/14169/

danbev · 2018-04-10T12:22:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/14174/

danbev · 2018-04-10T13:29:12Z

node-test-linux-linked-withoutintl failure looks unrelated

console output:

08:34:18 not ok 807 parallel/test-http-readable-data-event
08:34:18   ---
08:34:18   duration_ms: 0.895
08:34:18   severity: fail
08:34:18   stack: |-
08:34:18     assert.js:79
08:34:18       throw new AssertionError(obj);
08:34:18       ^
08:34:18     
08:34:18     AssertionError [ERR_ASSERTION]: 'Hello World!Hello again later!' strictEqual 'Hello World!'
08:34:18         at IncomingMessage.res.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/parallel/test-http-readable-data-event.js:43:14)
08:34:18         at IncomingMessage.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/common/index.js:467:15)
08:34:18         at IncomingMessage.emit (events.js:182:13)
08:34:18         at IncomingMessage.Readable.read (_stream_readable.js:489:10)
08:34:18         at IncomingMessage.res.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/parallel/test-http-readable-data-event.js:36:20)
08:34:18         at IncomingMessage.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/common/index.js:467:15)
08:34:18         at IncomingMessage.emit (events.js:182:13)
08:34:18         at emitReadable_ (_stream_readable.js:537:12)
08:34:18         at process._tickCallback (internal/process/next_tick.js:174:19)
08:34:18   ...

node-test-commit-linux-containered failure looks unrelated

console output:

08:34:18 not ok 807 parallel/test-http-readable-data-event
08:34:18   ---
08:34:18   duration_ms: 0.895
08:34:18   severity: fail
08:34:18   stack: |-
08:34:18     assert.js:79
08:34:18       throw new AssertionError(obj);
08:34:18       ^
08:34:18     
08:34:18     AssertionError [ERR_ASSERTION]: 'Hello World!Hello again later!' strictEqual 'Hello World!'
08:34:18         at IncomingMessage.res.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/parallel/test-http-readable-data-event.js:43:14)
08:34:18         at IncomingMessage.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/common/index.js:467:15)
08:34:18         at IncomingMessage.emit (events.js:182:13)
08:34:18         at IncomingMessage.Readable.read (_stream_readable.js:489:10)
08:34:18         at IncomingMessage.res.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/parallel/test-http-readable-data-event.js:36:20)
08:34:18         at IncomingMessage.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/test/common/index.js:467:15)
08:34:18         at IncomingMessage.emit (events.js:182:13)
08:34:18         at emitReadable_ (_stream_readable.js:537:12)
08:34:18         at process._tickCallback (internal/process/next_tick.js:174:19)
08:34:18   ...

danbev · 2018-04-11T06:21:47Z

Landed in 3989682.

Currently this test fails if run as the root user: Mismatched innerFn function calls. Expected exactly 62, actual 42. The motivation for this is that when building node in a docker container it is convenient to be able to build as root. PR-URL: #19850 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]>

Currently this test fails if run as the root user: Mismatched innerFn function calls. Expected exactly 62, actual 42. The motivation for this is that when building node in a docker container it is convenient to be able to build as root. PR-URL: nodejs#19850 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]>

nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 6, 2018

cjihrig approved these changes Apr 6, 2018

View reviewed changes

jasnell approved these changes Apr 6, 2018

View reviewed changes

BridgeAR approved these changes Apr 8, 2018

View reviewed changes

danbev added 2 commits April 10, 2018 06:23

test: add check for root user

3791b37

Currently this test fails if run as the root user: Mismatched innerFn function calls. Expected exactly 62, actual 42. The motivation for this is that when building node in a docker container it is convenient to be able to build as root.

squash: clean up

4768a54

danbev force-pushed the test-child-process-spawnsync-validation-root branch from 22c6cd6 to 4768a54 Compare April 10, 2018 05:22

richardlau approved these changes Apr 10, 2018

View reviewed changes

squash: add windows check to setting of rootUser

dcbcd5c

BridgeAR approved these changes Apr 10, 2018

View reviewed changes

danbev closed this Apr 11, 2018

danbev deleted the test-child-process-spawnsync-validation-root branch April 11, 2018 06:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add check for root user #19850

test: add check for root user #19850

danbev commented Apr 6, 2018

danbev commented Apr 6, 2018

danbev commented Apr 6, 2018

Trott commented Apr 6, 2018

danbev commented Apr 7, 2018

Trott commented Apr 7, 2018 •

edited

Loading

BridgeAR left a comment

BridgeAR Apr 8, 2018

danbev Apr 10, 2018

danbev commented Apr 10, 2018

danbev commented Apr 10, 2018

danbev commented Apr 10, 2018 •

edited

Loading

danbev commented Apr 11, 2018

test: add check for root user #19850

test: add check for root user #19850

Conversation

danbev commented Apr 6, 2018

Checklist

danbev commented Apr 6, 2018

danbev commented Apr 6, 2018

Trott commented Apr 6, 2018

danbev commented Apr 7, 2018

Trott commented Apr 7, 2018 • edited Loading

BridgeAR left a comment

Choose a reason for hiding this comment

BridgeAR Apr 8, 2018

Choose a reason for hiding this comment

danbev Apr 10, 2018

Choose a reason for hiding this comment

danbev commented Apr 10, 2018

danbev commented Apr 10, 2018

danbev commented Apr 10, 2018 • edited Loading

danbev commented Apr 11, 2018

Trott commented Apr 7, 2018 •

edited

Loading

danbev commented Apr 10, 2018 •

edited

Loading