Skip to content

test: fix flaky test-benchmark-events#17472

Closed
Trott wants to merge 1 commit intonodejs:masterfrom
Trott:fix-bench-events-test
Closed

test: fix flaky test-benchmark-events#17472
Trott wants to merge 1 commit intonodejs:masterfrom
Trott:fix-bench-events-test

Conversation

@Trott
Copy link
Member

@Trott Trott commented Dec 5, 2017

Make sure each benchmark file only ones one combination of options.
Allow for zero iterations for short benchmark.

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

test benchmark events

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 5, 2017
@Trott
Copy link
Member Author

Trott commented Dec 5, 2017

Example failure without these changes:

https://ci.nodejs.org/job/node-test-commit-linux/14693/nodes=alpine36-container-x64/consoleFull

not ok 148 parallel/test-benchmark-events
  ---
  duration_ms: 5.335
  severity: fail
  stack: |-
    
    events/ee-add-remove.js
    events/ee-add-remove.js n=1: 3,238.5412314876885
    
    events/ee-emit.js
    events/ee-emit.js listeners=1 argc=0 n=1: 8,308.684236764266
    events/ee-emit.js listeners=5 argc=0 n=1: 7,649.763239827728
    events/ee-emit.js listeners=10 argc=0 n=1: 7,807.681196761374
    events/ee-emit.js listeners=1 argc=2 n=1: 7,828.52401008314
    events/ee-emit.js listeners=5 argc=2 n=1: 16,525.10162937502
    events/ee-emit.js listeners=10 argc=2 n=1: 8,180.226755885673
    events/ee-emit.js listeners=1 argc=4 n=1: 21,679.276779326643
    /home/iojs/build/workspace/node-test-commit-linux/nodes/alpine36-container-x64/benchmark/common.js:202
          throw new Error('insufficient clock precision for short benchmark');
          ^
    
    Error: insufficient clock precision for short benchmark
        at Benchmark.end (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine36-container-x64/benchmark/common.js:202:13)
        at main (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine36-container-x64/benchmark/events/ee-emit.js:35:13)
        at Benchmark.process.nextTick (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine36-container-x64/benchmark/common.js:34:28)
        at process._tickCallback (internal/process/next_tick.js:155:11)
        at Function.Module.runMain (module.js:703:11)
        at startup (bootstrap_node.js:195:16)
        at bootstrap_node.js:646:3
    assert.js:42
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: 1 strictEqual 0
        at ChildProcess.child.on (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine36-container-x64/test/common/benchmark.js:25:12)
        at ChildProcess.emit (events.js:126:13)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:209:12)
  ...

@Trott Trott added benchmark Issues and PRs related to the benchmark subsystem. events Issues and PRs related to the events subsystem / EventEmitter. labels Dec 5, 2017
@Trott
Copy link
Member Author

Trott commented Dec 5, 2017

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

@Trott
Copy link
Member Author

Trott commented Dec 5, 2017

CI failure is no doubt unrelated build issue but here's a re-run: https://ci.nodejs.org/job/node-test-commit-linux-linked/609/

mhdawson
mhdawson approved these changes Dec 5, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

Typo in commit comment:

'only ones one combination of options'

otherwise LGTM

@Trott
test: fix flaky test-benchmark-events
394ace6 
Make sure each benchmark file only runs one combination of options.
Allow for zero iterations for short benchmark.
@Trott Trott force-pushed the fix-bench-events-test branch from 9ed8fb8 to 394ace6 Compare December 5, 2017 19:05
@Trott
Copy link
Member Author

Trott commented Dec 5, 2017

Typo in commit fixed. Thanks, @mhdawson!

@Trott
Copy link
Member Author

Trott commented Dec 5, 2017

(CI is good.)

@BridgeAR
Copy link
Member

BridgeAR commented Dec 6, 2017

I think this can land below the 48 hours.

@Trott
Copy link
Member Author

Trott commented Dec 6, 2017

Landed in b3e5367

@Trott Trott closed this Dec 6, 2017
Trott added a commit to Trott/io.js that referenced this pull request Dec 6, 2017
@Trott
test: fix flaky test-benchmark-events
b3e5367 
Make sure each benchmark file only runs one combination of options.
Allow for zero iterations for short benchmark.

PR-URL: nodejs#17472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@Trott @MylesBorins
test: fix flaky test-benchmark-events
71be0a4 
Make sure each benchmark file only runs one combination of options.
Allow for zero iterations for short benchmark.

PR-URL: #17472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@Trott @MylesBorins
test: fix flaky test-benchmark-events
619cbc4 
Make sure each benchmark file only runs one combination of options.
Allow for zero iterations for short benchmark.

PR-URL: #17472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@Trott @gibfahn
test: fix flaky test-benchmark-events
d12d86e 
Make sure each benchmark file only runs one combination of options.
Allow for zero iterations for short benchmark.

PR-URL: #17472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@Trott @gibfahn
test: fix flaky test-benchmark-events
d0738fe 
Make sure each benchmark file only runs one combination of options.
Allow for zero iterations for short benchmark.

PR-URL: #17472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
@Trott Trott deleted the fix-bench-events-test branch January 13, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@mhdawson mhdawson mhdawson approved these changes

Assignees

No one assigned

Labels

benchmark Issues and PRs related to the benchmark subsystem. events Issues and PRs related to the events subsystem / EventEmitter. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott @BridgeAR @jasnell @cjihrig @mhdawson @gibfahn @nodejs-github-bot