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: chdir before running test-cli-node-options #12660

Closed

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 26, 2017

When test-cli-node-options is run it uses the --trace-events-enabled
option which generates a file named node_trace.1.log. This commit
changes the working directory to the test tmp directory to avoid this
file being created in the project root.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 26, 2017
@danbev
Copy link
Contributor Author

danbev commented Apr 26, 2017

@hiroppy hiroppy added the cli Issues and PRs related to the Node.js command line interface. label Apr 26, 2017
@danbev
Copy link
Contributor Author

danbev commented Apr 26, 2017

test/linux failure looks unrelated to me:

not ok 1370 sequential/test-benchmark-http
  ---
  duration_ms: 3.449
  severity: fail
  stack: |-
    
    http/bench-parser.js
    http/bench-parser.js n=1 len=1: 2,420.779975308044
    
    http/check_invalid_header_char.js
    http/check_invalid_header_char.js n=1 key="": 4,056.219198085465
    http/check_invalid_header_char.js n=1 key="1": 3,417.226237463051
    http/check_invalid_header_char.js n=1 key="\t\t\t\t\t\t\t\t\t\tFoo bar baz": 3,887.5563209722
    http/check_invalid_header_char.js n=1 key="keep-alive": 3,926.434326459456
    http/check_invalid_header_char.js n=1 key="close": 3,807.0582860623595
    http/check_invalid_header_char.js n=1 key="gzip": 3,452.5856413868346
    http/check_invalid_header_char.js n=1 key="20091": 3,292.647189560991
    http/check_invalid_header_char.js n=1 key="private": 3,483.1918576907137
    http/check_invalid_header_char.js n=1 key="text/html; charset=utf-8": 3,870.3285134842245
    http/check_invalid_header_char.js n=1 key="text/plain": 3,708.401011651796
    http/check_invalid_header_char.js n=1 key="Sat, 07 May 2016 16:54:48 GMT": 3,227.1597766805435
    http/check_invalid_header_char.js n=1 key="SAMEORIGIN": 3,907.013088493846
    http/check_invalid_header_char.js n=1 key="en-US": 3,414.6008331626035
    http/check_invalid_header_char.js n=1 key="Here is a value that is really a folded header value\r\n  this should be      supported, but it is not currently": 3,331.8895145436977
    http/check_invalid_header_char.js n=1 key="中文呢": 3,943.295411975788
    http/check_invalid_header_char.js n=1 key="foo\nbar": 3,672.015569346014
    http/check_invalid_header_char.js n=1 key="�": 4,076.8901481949574
    
    http/check_is_http_token.js
    http/check_is_http_token.js n=1 key="TCN": 4,408.490753190646
    http/check_is_http_token.js n=1 key="ETag": 4,293.227433723301
    http/check_is_http_token.js n=1 key="date": 4,371.240732969646
    http/check_is_http_token.js n=1 key="Vary": 2,659.652277061297
    http/check_is_http_token.js n=1 key="server": 4,318.367311686798
    http/check_is_http_token.js n=1 key="Server": 4,250.363406071219
    http/check_is_http_token.js n=1 key="status": 2,260.7351006253193
    http/check_is_http_token.js n=1 key="version": 1,966.4291220287257
    http/check_is_http_token.js n=1 key="Expires": 4,192.0635852204605
    http/check_is_http_token.js n=1 key="alt-svc": 4,076.590991549227
    /home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/run.js:62
          var rate = data.rate.toString().split('.');
                              ^
    
    TypeError: Cannot read property 'toString' of null
        at ChildProcess.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/run.js:62:27)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at process.nextTick (internal/child_process.js:753:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:7)
        at process._tickCallback (internal/process/next_tick.js:105:9)
    
    assert.js:86
      throw new assert.AssertionError({
      ^
    AssertionError: 1 === 0
        at ChildProcess.child.on (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/test/sequential/test-benchmark-http.js:32:10)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:196:12)
    events.js:182
          throw er; // Unhandled 'error' event
          ^
    
    Error: channel closed
        at process.target.send (internal/child_process.js:549:16)
        at ChildProcess.sendResult (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/common.js:221:13)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at process.nextTick (internal/child_process.js:753:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:7)
        at process._tickCallback (internal/process/next_tick.js:105:9)
  ...

Makefile:349: recipe for target 'test-ci' failed
make[1]: *** [test-ci] Error 1
make[1]: Leaving directory '/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24'
Makefile:511: recipe for target 'run-ci' failed
make: *** [run-ci] Error 2
Build step 'Execute shell' marked build as failure
Run condition [Always] enabling perform for step [[Execute a set of scripts]]
Run condition [Always] enabling perform for step [[Execute a set of scripts]]
TAP Reports Processing: START
Looking for TAP results report in workspace using pattern: *.tap
Saving reports...
Processing '/var/lib/jenkins/jobs/node-test-commit-linux/configurations/axis-nodes/fedora24/builds/9421/tap-master-files/cctest.tap'
Parsing TAP test result [/var/lib/jenkins/jobs/node-test-commit-linux/configurations/axis-nodes/fedora24/builds/9421/tap-master-files/cctest.tap].
Processing '/var/lib/jenkins/jobs/node-test-commit-linux/configurations/axis-nodes/fedora24/builds/9421/tap-master-files/test.tap'
Parsing TAP test result [/var/lib/jenkins/jobs/node-test-commit-linux/configurations/axis-nodes/fedora24/builds/9421/tap-master-files/test.tap].
org.tap4j.parser.ParserException: Error parsing TAP Stream: Error parsing YAML [  duration_ms: 3.449
  severity: fail
  stack: |-
    
    http/bench-parser.js
    http/bench-parser.js n=1 len=1: 2,420.779975308044
    
    http/check_invalid_header_char.js
    http/check_invalid_header_char.js n=1 key="": 4,056.219198085465
    http/check_invalid_header_char.js n=1 key="1": 3,417.226237463051
    http/check_invalid_header_char.js n=1 key="\t\t\t\t\t\t\t\t\t\tFoo bar baz": 3,887.5563209722
    http/check_invalid_header_char.js n=1 key="keep-alive": 3,926.434326459456
    http/check_invalid_header_char.js n=1 key="close": 3,807.0582860623595
    http/check_invalid_header_char.js n=1 key="gzip": 3,452.5856413868346
    http/check_invalid_header_char.js n=1 key="20091": 3,292.647189560991
    http/check_invalid_header_char.js n=1 key="private": 3,483.1918576907137
    http/check_invalid_header_char.js n=1 key="text/html; charset=utf-8": 3,870.3285134842245
    http/check_invalid_header_char.js n=1 key="text/plain": 3,708.401011651796
    http/check_invalid_header_char.js n=1 key="Sat, 07 May 2016 16:54:48 GMT": 3,227.1597766805435
    http/check_invalid_header_char.js n=1 key="SAMEORIGIN": 3,907.013088493846
    http/check_invalid_header_char.js n=1 key="en-US": 3,414.6008331626035
    http/check_invalid_header_char.js n=1 key="Here is a value that is really a folded header value\r\n  this should be      supported, but it is not currently": 3,331.8895145436977
    http/check_invalid_header_char.js n=1 key="中文呢": 3,943.295411975788
    http/check_invalid_header_char.js n=1 key="foo\nbar": 3,672.015569346014
    http/check_invalid_header_char.js n=1 key="�": 4,076.8901481949574
    
    http/check_is_http_token.js
    http/check_is_http_token.js n=1 key="TCN": 4,408.490753190646
    http/check_is_http_token.js n=1 key="ETag": 4,293.227433723301
    http/check_is_http_token.js n=1 key="date": 4,371.240732969646
    http/check_is_http_token.js n=1 key="Vary": 2,659.652277061297
    http/check_is_http_token.js n=1 key="server": 4,318.367311686798
    http/check_is_http_token.js n=1 key="Server": 4,250.363406071219
    http/check_is_http_token.js n=1 key="status": 2,260.7351006253193
    http/check_is_http_token.js n=1 key="version": 1,966.4291220287257
    http/check_is_http_token.js n=1 key="Expires": 4,192.0635852204605
    http/check_is_http_token.js n=1 key="alt-svc": 4,076.590991549227
    /home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/run.js:62
          var rate = data.rate.toString().split('.');
                              ^
    
    TypeError: Cannot read property 'toString' of null
        at ChildProcess.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/run.js:62:27)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at process.nextTick (internal/child_process.js:753:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:7)
        at process._tickCallback (internal/process/next_tick.js:105:9)
    
    assert.js:86
      throw new assert.AssertionError({
      ^
    AssertionError: 1 === 0
        at ChildProcess.child.on (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/test/sequential/test-benchmark-http.js:32:10)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:196:12)
    events.js:182
          throw er; // Unhandled 'error' event
          ^
    
    Error: channel closed
        at process.target.send (internal/child_process.js:549:16)
        at ChildProcess.sendResult (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/common.js:221:13)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at process.nextTick (internal/child_process.js:753:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:7)
        at process._tickCallback (internal/process/next_tick.js:105:9)
]: special characters are not allowed
	at org.tap4j.parser.Tap13Parser.parseTapStream(Tap13Parser.java:230)
	at org.tap4j.parser.Tap13Parser.parseFile(Tap13Parser.java:193)
	at org.tap4j.plugin.TapParser.parse(TapParser.java:167)
	at org.tap4j.plugin.TapPublisher.loadResults(TapPublisher.java:540)
	at org.tap4j.plugin.TapPublisher.performImpl(TapPublisher.java:412)
	at org.tap4j.plugin.TapPublisher.perform(TapPublisher.java:371)
	at hudson.tasks.BuildStepCompatibilityLayer.perform(BuildStepCompatibilityLayer.java:78)
	at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
	at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:779)
	at hudson.model.AbstractBuild$AbstractBuildExecution.performAllBuildSteps(AbstractBuild.java:720)
	at hudson.model.Build$BuildExecution.post2(Build.java:186)
	at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:665)
	at hudson.model.Run.execute(Run.java:1753)
	at hudson.matrix.MatrixRun.run(MatrixRun.java:146)
	at hudson.model.ResourceController.execute(ResourceController.java:98)
	at hudson.model.Executor.run(Executor.java:405)
Caused by: org.tap4j.parser.ParserException: Error parsing YAML [  duration_ms: 3.449
  severity: fail
  stack: |-
    
    http/bench-parser.js
    http/bench-parser.js n=1 len=1: 2,420.779975308044
    
    http/check_invalid_header_char.js
    http/check_invalid_header_char.js n=1 key="": 4,056.219198085465
    http/check_invalid_header_char.js n=1 key="1": 3,417.226237463051
    http/check_invalid_header_char.js n=1 key="\t\t\t\t\t\t\t\t\t\tFoo bar baz": 3,887.5563209722
    http/check_invalid_header_char.js n=1 key="keep-alive": 3,926.434326459456
    http/check_invalid_header_char.js n=1 key="close": 3,807.0582860623595
    http/check_invalid_header_char.js n=1 key="gzip": 3,452.5856413868346
    http/check_invalid_header_char.js n=1 key="20091": 3,292.647189560991
    http/check_invalid_header_char.js n=1 key="private": 3,483.1918576907137
    http/check_invalid_header_char.js n=1 key="text/html; charset=utf-8": 3,870.3285134842245
    http/check_invalid_header_char.js n=1 key="text/plain": 3,708.401011651796
    http/check_invalid_header_char.js n=1 key="Sat, 07 May 2016 16:54:48 GMT": 3,227.1597766805435
    http/check_invalid_header_char.js n=1 key="SAMEORIGIN": 3,907.013088493846
    http/check_invalid_header_char.js n=1 key="en-US": 3,414.6008331626035
    http/check_invalid_header_char.js n=1 key="Here is a value that is really a folded header value\r\n  this should be      supported, but it is not currently": 3,331.8895145436977
    http/check_invalid_header_char.js n=1 key="中文呢": 3,943.295411975788
    http/check_invalid_header_char.js n=1 key="foo\nbar": 3,672.015569346014
    http/check_invalid_header_char.js n=1 key="�": 4,076.8901481949574
    
    http/check_is_http_token.js
    http/check_is_http_token.js n=1 key="TCN": 4,408.490753190646
    http/check_is_http_token.js n=1 key="ETag": 4,293.227433723301
    http/check_is_http_token.js n=1 key="date": 4,371.240732969646
    http/check_is_http_token.js n=1 key="Vary": 2,659.652277061297
    http/check_is_http_token.js n=1 key="server": 4,318.367311686798
    http/check_is_http_token.js n=1 key="Server": 4,250.363406071219
    http/check_is_http_token.js n=1 key="status": 2,260.7351006253193
    http/check_is_http_token.js n=1 key="version": 1,966.4291220287257
    http/check_is_http_token.js n=1 key="Expires": 4,192.0635852204605
    http/check_is_http_token.js n=1 key="alt-svc": 4,076.590991549227
    /home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/run.js:62
          var rate = data.rate.toString().split('.');
                              ^
    
    TypeError: Cannot read property 'toString' of null
        at ChildProcess.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/run.js:62:27)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at process.nextTick (internal/child_process.js:753:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:7)
        at process._tickCallback (internal/process/next_tick.js:105:9)
    
    assert.js:86
      throw new assert.AssertionError({
      ^
    AssertionError: 1 === 0
        at ChildProcess.child.on (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/test/sequential/test-benchmark-http.js:32:10)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:196:12)
    events.js:182
          throw er; // Unhandled 'error' event
          ^
    
    Error: channel closed
        at process.target.send (internal/child_process.js:549:16)
        at ChildProcess.sendResult (/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/benchmark/common.js:221:13)
        at emitTwo (events.js:125:13)
        at ChildProcess.emit (events.js:213:7)
        at process.nextTick (internal/child_process.js:753:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:7)
        at process._tickCallback (internal/process/next_tick.js:105:9)
]: special characters are not allowed
	at org.tap4j.parser.Tap13Parser.parseDiagnostics(Tap13Parser.java:435)
	at org.tap4j.parser.Tap13Parser.parseLine(Tap13Parser.java:262)
	at org.tap4j.parser.Tap13Parser.parseTapStream(Tap13Parser.java:224)
	... 15 more
Caused by: unacceptable character '�' (0x7F) special characters are not allowed
in "'string'", position 1598
	at org.yaml.snakeyaml.reader.StreamReader.checkPrintable(StreamReader.java:67)
	at org.yaml.snakeyaml.reader.StreamReader.<init>(StreamReader.java:47)
	at org.yaml.snakeyaml.Yaml.load(Yaml.java:369)
	at org.tap4j.parser.Tap13Parser.parseDiagnostics(Tap13Parser.java:430)
	... 17 more
TAP parse errors found in the build. Marking build as UNSTABLE
TAP Reports Processing: FINISH
Checking ^not ok
/home/iojs/build/workspace/node-test-commit-linux/nodes/fedora24/test.tap:
not ok 1370 sequential/test-benchmark-http
Notifying upstream projects of job completion
Finished: FAILURE

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

I'd prefer something like this early in the test file:

common.refreshTmpDir();
require('process').chdir(common.tmpDir);

Then, the trace file ends up in the temp directory and doesn't need to be cleaned up. Moreover, a test writing files into the project root makes me uncomfortable.

(The refreshTmpDir() call is probably not strictly needed, but it's probably a good idea to always refresh the tmp directory in any tests that use it.) Correcting myself: Actually, it is necessary because there's a chance the directory doesn't exist and that will create it.

@danbev
Copy link
Contributor Author

danbev commented Apr 27, 2017

'd prefer something like this early in the test file

That makes sense. I've updated the PR now. Thanks

@danbev
Copy link
Contributor Author

danbev commented Apr 27, 2017

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

LGTM if the commit message is updated to match the new behavior.

When test-cli-node-options is run it uses the --trace-events-enabled
option which generates a file named node_trace.1.log. This commit
changes the working directory to the test tmp directory to avoid this
file being created in the project root.
@danbev danbev force-pushed the cleanup-after-test-cli-node-options branch from 273810b to e347255 Compare April 27, 2017 11:01
@danbev danbev changed the title test: remove node_trace.1.log file after test test: chdir before running test-cli-node-options Apr 27, 2017
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green. Thanks!

@refack
Copy link
Contributor

refack commented Apr 27, 2017

@danbev I stopped one build on windows that was running for 8 hour https://ci.nodejs.org/job/node-compile-windows/8562/label=win-vs2015-x86/
IMHO it's safe to ignore (other Windows build pass)

@danbev
Copy link
Contributor Author

danbev commented Apr 27, 2017

@refack Ah great, thanks

@danbev
Copy link
Contributor Author

danbev commented Apr 27, 2017

danbev added a commit to danbev/node that referenced this pull request Apr 28, 2017
When test-cli-node-options is run it uses the --trace-events-enabled
option which generates a file named node_trace.1.log. This commit
changes the working directory to the test tmp directory to avoid this
file being created in the project root.

PR-URL: nodejs#12660
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Apr 28, 2017

Landed in a4fd9e5

@danbev danbev closed this Apr 28, 2017
@jasnell jasnell mentioned this pull request May 11, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

Should land with #12677

@danbev danbev deleted the cleanup-after-test-cli-node-options branch June 28, 2017 05:26
sam-github pushed a commit to sam-github/node that referenced this pull request Oct 11, 2017
When test-cli-node-options is run it uses the --trace-events-enabled
option which generates a file named node_trace.1.log. This commit
changes the working directory to the test tmp directory to avoid this
file being created in the project root.

PR-URL: nodejs#12660
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 16, 2017
When test-cli-node-options is run it uses the --trace-events-enabled
option which generates a file named node_trace.1.log. This commit
changes the working directory to the test tmp directory to avoid this
file being created in the project root.

Backport-PR-URL: #12677
PR-URL: #12660
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 17, 2017
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
When test-cli-node-options is run it uses the --trace-events-enabled
option which generates a file named node_trace.1.log. This commit
changes the working directory to the test tmp directory to avoid this
file being created in the project root.

Backport-PR-URL: #12677
PR-URL: #12660
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues and PRs related to the Node.js command line interface. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants