GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||
GeezerVison is showing github.com — benchmark: test refactoring by BridgeAR · Pull Request #31396 · nodejs/node Conversation
|
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@nodejs/http2 @ronag I had to end the process hard in case this runs the http2 benchmarks. Maybe someone could have a look what part stays alive.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
1cb7b50 to
27eee27
Compare
|
This is ready for review. I just cleaned up the commits. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/11672/ ✔️ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This does reduce the test runtime for almost all benchmarks. Mostly by a few hundred milliseconds. The runtime for |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This could use another review @nodejs/benchmarking |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
27eee27 to
3fd40c6
Compare
|
Needs a rebase. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Rubber-stamp LGTM. The concept is sound.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
3fd40c6 to
6ea89f5
Compare
Sorry, something went wrong.
There was an error while loading. Please reload this page.
9bdb710 to
f7926f3
Compare
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This is the basis to refactor the helper to use modern class language features such as private fields. It also refactors the exports to use module.exports. That way it's immediately clear what parts are exported.
This reverts commit b70741e. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
There was an error while loading. Please reload this page.
|
Also something that I just now noticed is that this PR changed the formatting of the configuration string (when writing the CSV output) such that each parameter is preceded by a space whether it's needed or not, which breaks tooling (something I'm currently working on) that monitors changes in benchmark results by comparing results for a particular configuration. Just something to keep in mind if these changes get merged again later on. EDIT: FWIW I'm now incorporating extra safeguards to prevent this sort of thing in my tooling in the future... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This is the basis to refactor the helper to use modern class language features such as private fields. It also refactors the exports to use module.exports. That way it's immediately clear what parts are exported. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This adds a new `test` option. Using it automatically uses a single minimal option matrix to verify the benchmark works as expected. Using the new `all` option makes sure all test suites are run. On top of that the benchmarks will from now on report properly what category might have a typo, if any. The http duration was also refactored to use a option instead of relying on a configuration setting. The fixture folder is ignored as test suite from now on. PR-URL: #31396 Fixes: #31083 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
It was necessary to have fallbacks to run the original tests. This is obsolete with the new test mode. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This reverts commit 357230f. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit 78aa348. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit dac5795. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit b70741e. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
|
@BridgeAR could you please manually backport this to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This is the basis to refactor the helper to use modern class language features such as private fields. It also refactors the exports to use module.exports. That way it's immediately clear what parts are exported. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
It was necessary to have fallbacks to run the original tests. This is obsolete with the new test mode. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This is the basis to refactor the helper to use modern class language features such as private fields. It also refactors the exports to use module.exports. That way it's immediately clear what parts are exported. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
It was necessary to have fallbacks to run the original tests. This is obsolete with the new test mode. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This is the basis to refactor the helper to use modern class language features such as private fields. It also refactors the exports to use module.exports. That way it's immediately clear what parts are exported. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
It was necessary to have fallbacks to run the original tests. This is obsolete with the new test mode. PR-URL: #31396 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This reverts commit 357230f. Refs: nodejs#31396 PR-URL: nodejs#31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit b70741e. Refs: nodejs#31396 PR-URL: nodejs#31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit 357230f. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit b70741e. Refs: #31396 PR-URL: #31722 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Benchmarks should expose their options
There was an error while loading. Please reload this page.