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

benchmark: fix and extend assert benchmarks #14147

Closed
wants to merge 1 commit into from
Closed

benchmark: fix and extend assert benchmarks #14147

wants to merge 1 commit into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Jul 9, 2017

The benchmarks had the strict and non strict labels switched.
This is fixed and the benchmarks were extended to check more
possible input types and function calls.

Ref: #13973

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

benchmark

@BridgeAR
benchmark: fix and extend assert benchmarks
4ce4298 
The benchmarks had the strict and non strict labels switched.
This is fixed and the benchmarks were extended to check more
possible input types and function calls.
@nodejs-github-bot nodejs-github-bot added assert Issues and PRs related to the assert subsystem. benchmark Issues and PRs related to the benchmark subsystem. labels Jul 9, 2017
@refack
Copy link
Contributor

refack commented Jul 9, 2017

/cc @nodejs/benchmarking

refack
refack approved these changes Jul 9, 2017
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

LGTM, but need approval from @nodejs/benchmarking since this will perturb https://benchmarking.nodejs.org/

@refack refack self-assigned this Jul 9, 2017
@BridgeAR
Copy link
Member Author

PTAL

jasnell
jasnell approved these changes Jul 19, 2017
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

largely rubber stamp LGTM. nothing stands out as being problematic but I'd like @mscdex and @nodejs/benchmarking to take a look

@refack
Copy link
Contributor

refack commented Jul 19, 2017

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

@refack refack removed their assignment Jul 20, 2017
@refack refack mentioned this pull request Jul 22, 2017
Closed
4 tasks
@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 8, 2017

PTAL

@refack
Copy link
Contributor

refack commented Aug 13, 2017

I'm taking silence as acceptance...
Pre-land CI: https://ci.nodejs.org/job/node-test-pull-request/9639/

@refack refack self-assigned this Aug 13, 2017
@refack
Copy link
Contributor

refack commented Aug 13, 2017 •
edited
Loading

Landed in a7189c0
Extra sanity on master: https://ci.nodejs.org/job/node-test-commit-linuxone/7941/ ✔️

@refack refack closed this Aug 13, 2017
refack pushed a commit that referenced this pull request Aug 13, 2017
@BridgeAR @refack
benchmark: fix and extend assert benchmarks
a7189c0 
The benchmarks had the strict and non strict labels switched.
This is fixed and the benchmarks were extended to check more
possible input types and function calls.

PR-URL: #14147
Refs: #13973
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@refack refack mentioned this pull request Aug 13, 2017
Closed
2 tasks
addaleax pushed a commit that referenced this pull request Aug 13, 2017
@BridgeAR @addaleax
benchmark: fix and extend assert benchmarks
b3c1c6f 
The benchmarks had the strict and non strict labels switched.
This is fixed and the benchmarks were extended to check more
possible input types and function calls.

PR-URL: #14147
Refs: #13973
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax addaleax mentioned this pull request Aug 13, 2017
Merged
@refack refack removed their assignment Oct 20, 2018
@BridgeAR BridgeAR deleted the improve-assert-benchmarks branch April 1, 2019 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
assert Issues and PRs related to the assert subsystem. benchmark Issues and PRs related to the benchmark subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@BridgeAR @refack @jasnell @MylesBorins @nodejs-github-bot