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

Run benchmark test suite nightly #1568

Closed
Trott opened this issue Nov 11, 2018 · 11 comments
Closed

Run benchmark test suite nightly #1568

Trott opened this issue Nov 11, 2018 · 11 comments
Assignees
@refack
Labels
ci-change PSA of configuration changes ci-job-request
Projects
action items

Comments

@Trott
Copy link
Member

Trott commented Nov 11, 2018

Similar to the way we run internet nightly or maybe made part of node-daily-master, the benchmark tests need to be incorporated into a once-a-day run somehow. They've been moved out of the main test suite.

Refs: nodejs/node#24265
@refack
Copy link
Contributor

refack commented Nov 11, 2018 •
edited

Done.
https://ci.nodejs.org/job/node-test-commit-custom-suites/753/

@refack
Copy link
Contributor

refack commented Nov 11, 2018

There is an issue with the napi benchmark, since the current job doesn't trigger it's build.

@refack refack self-assigned this Nov 11, 2018
@refack refack added ci-job-request ci-change PSA of configuration changes labels Nov 11, 2018
@refack refack added this to untriaged in action items via automation Nov 11, 2018
@Trott
Copy link
Member Author

Trott commented Nov 11, 2018

There is an issue with the napi benchmark, since the current job doesn't trigger it's build.

Oh, because it doesn't run make test-benchmark, it just directly calls the benchmark tests with tools/test.py. Hmmm.... any recommendations for how to fix?

@refack
Copy link
Contributor

refack commented Nov 11, 2018

either restore https://github.com/nodejs/node/pull/24265/files#diff-b67911656ef5d18c4ae36cb6741b7965L458
or put an if (benchmark in CI_JS_SUITES) (pseudocode) around it

@refack
Copy link
Contributor

refack commented Nov 11, 2018

or put a guard in benchmark/napi/function_args/index.js that urges you to run make bench-addons-build

@joyeecheung
Copy link
Member

joyeecheung commented Nov 11, 2018 •
edited

or put a guard in benchmark/napi/function_args/index.js that urges you to run make bench-addons-build

@refack I think I've attempted to do that a while back but I couldn't make child_process play along with node-gyp arguments..(but limited that to make may work?)

@Trott
Copy link
Member Author

Trott commented Nov 11, 2018

I like restoring the one line in https://github.com/nodejs/node/pull/24265/files#diff-b67911656ef5d18c4ae36cb6741b7965L458. While it compiles a file unnecessarily in most cases, it's quick.

@joyeecheung
Copy link
Member

Ah, I think I've managed to get it to work in JS this time around..(don't really know why I didn't, maybe I was too sleep-deprived) (considering there is tools/build-addons.js there is no reason it doesn't work I guess)

Trott added a commit to Trott/io.js that referenced this issue Nov 11, 2018
@Trott
build: fix benchmark tests on CI
792cae0 
Refs: nodejs/build#1568 (comment)
@Trott Trott mentioned this issue Nov 11, 2018
Closed
2 tasks
@Trott
Copy link
Member Author

Trott commented Nov 11, 2018

Ah, I think I've managed to get it to work in JS this time around..(don't really know why I didn't, maybe I was too sleep-deprived) (considering there is tools/build-addons.js there is no reason it doesn't work I guess)

Oh, I just opened nodejs/node#24307. Feel free to close it if you have a better solution!

@joyeecheung joyeecheung mentioned this issue Nov 11, 2018
Closed
3 tasks
@refack
Copy link
Contributor

refack commented Nov 11, 2018

I think nodejs/node#24307 is the simplest solution. We can flip it again as part of nodejs/node#24309.

@refack refack moved this from untriaged to In Progress in action items Nov 11, 2018
danbev pushed a commit to nodejs/node that referenced this issue Nov 12, 2018
@Trott @danbev
build: fix benchmark tests on CI
f697457 
PR-URL: #24307
Refs:
nodejs/build#1568 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
BridgeAR pushed a commit to nodejs/node that referenced this issue Nov 14, 2018
@Trott @BridgeAR
build: fix benchmark tests on CI
44ebdbb 
PR-URL: #24307
Refs:
nodejs/build#1568 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this issue Nov 15, 2018
@Trott @kiyomizumia
build: fix benchmark tests on CI
6f603ab 
PR-URL: nodejs#24307
Refs:
nodejs/build#1568 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@Trott
Copy link
Member Author

Trott commented Dec 7, 2018

I believe this is resolved.

@Trott Trott closed this as completed Dec 7, 2018
action items automation moved this from In Progress to done Dec 7, 2018
codebytere pushed a commit to nodejs/node that referenced this issue Jan 12, 2019
@Trott @codebytere
build: fix benchmark tests on CI
33fd13c 
PR-URL: #24307
Refs:
nodejs/build#1568 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@richardlau richardlau mentioned this issue Jul 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@refack refack

Labels
ci-change PSA of configuration changes ci-job-request
Projects
action items
done
Milestone
No milestone
Development

No branches or pull requests
3 participants
@refack @Trott @joyeecheung