-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
benchmark: add benchmarks for the test_runner #48931
benchmark: add benchmarks for the test_runner #48931
Conversation
}); | ||
} | ||
|
||
await Promise.all(promises); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use finished
on the reporter or this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly didn't understand what we are benchmarking. Can you leave some descriptive comments on what we are benchmarking? PS: The file names does not help either. (For example: plain-tests-as-fast-as-can)
we want to compare how changes in test runner code affect its performance, but I agree naming can be improved @rluvaton in regarding to #47945 (comment) - @cjihrig and myself were talking about what we want to benchmark - we probably also want to run tests with |
I don't think running the test-runner fixture folder is a good idea as it changes, while I think benchmarks are best when they are static
I avoided that on purpose to reduce the scope of the benchmark, adding the |
it is ok since the benchmarks are compared before and after a change we want to understand the impact of. no baseline or benchmark is saved according to @cjihrig
that is a good point, but I think that should only mean we should rely on this specific benchmark only if it is significantly different, not that we shouldn't test it at all |
I prefer if you add those as you also need to add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Landed in f458e5b |
PR-URL: nodejs#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: #48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: #48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: #48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs/node#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs/node#48931 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
I'm trying to benchmark tests creation and execution
this is needed as we need to evaluate how a change in the test runner affects the performance: