Skip to content

fix: skip failing benchmarks #1612

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

Merged
merged 1 commit into from
Jan 31, 2025
Merged

fix: skip failing benchmarks #1612

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,24 @@ async function main() {

console.log('Executing "%s"', c);

childProcess.execFileSync(cmd[0], cmd.slice(1), {
shell: false,
stdio: 'inherit',
});
try {
childProcess.execFileSync(cmd[0], cmd.slice(1), {
shell: false,
stdio: 'inherit',
});
} catch (e) {
// See #1611.
// Due to the wide range of modules and node versions that we
// benchmark these days, not every library supports every node
// version.
// So we ignore any benchmark that fails in order to not fail the
// whole run benchmark gh action. Their results will not be
// visible for this node version in the frontend.
// The better solution would be benchmark case metadata that lists
// compatible / node versions (e.g. for stnl sth like >= 23) and
// then skip incompatible node versions explicitly.
console.error('Skipped "%s" benchmark due to an error', c);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.error('Skipped "%s" benchmark due to an error', c);
console.error('Skipped "%s" benchmark due to an error', c, e);

might be better to print error messages for easier debugging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch but this only silences the exception thrown by childProcess.execFileSync. The original error in the subprocess is still printed to stdout or stderr:

erik@t460:~/src/typescript-runtime-type-benchmarks (fix-skip-errors-during-benchmark)$ nvm use 16
Now using node v16.20.1 (npm v8.19.4)
erik@t460:~/src/typescript-runtime-type-benchmarks (fix-skip-errors-during-benchmark)$ npm run start run stnl

> typescript-runtime-type-benchmarks@1.0.0 start
> ts-node index.ts run stnl

Removing previous results
Executing "stnl"
Loading "stnl"
/home/erik/src/typescript-runtime-type-benchmarks/node_modules/ts-node/dist/index.js:851
            return old(m, filename);
                   ^
Error [ERR_REQUIRE_ESM]: require() of ES Module /home/erik/src/typescript-runtime-type-benchmarks/node_modules/stnl/compilers/validate-json/compose.js from /home/erik/src/typescript-runtime-type-benchmarks/cases/stnl.ts not supported.
Instead change the require of compose.js in /home/erik/src/typescript-runtime-type-benchmarks/cases/stnl.ts to a dynamic import() which is available in all CommonJS modules.
    at Object.require.extensions.<computed> [as .js] (/home/erik/src/typescript-runtime-type-benchmarks/node_modules/ts-node/dist/index.js:851:20)
    at Object.<anonymous> (/home/erik/src/typescript-runtime-type-benchmarks/cases/stnl.ts:6:35)
    at Module.m._compile (/home/erik/src/typescript-runtime-type-benchmarks/node_modules/ts-node/dist/index.js:857:29)
    at Object.require.extensions.<computed> [as .ts] (/home/erik/src/typescript-runtime-type-benchmarks/node_modules/ts-node/dist/index.js:859:16)
    at /home/erik/src/typescript-runtime-type-benchmarks/cases/index.ts:87:72
    at async Object.importCase (/home/erik/src/typescript-runtime-type-benchmarks/cases/index.ts:87:5)
    at async main (/home/erik/src/typescript-runtime-type-benchmarks/index.ts:115:21) {
  code: 'ERR_REQUIRE_ESM'
}
Skipped "stnl" benchmark due to an error

This is the exception that gets ignored:

Error: Command failed: /home/erik/src/typescript-runtime-type-benchmarks/node_modules/.bin/ts-node /home/erik/src/typescript-runtime-type-benchmarks/index.ts run-internal stnl
    at genericNodeError (node:internal/errors:984:15)
    at wrappedFn (node:internal/errors:538:14)
    at checkExecSyncError (node:child_process:890:11)
    at Object.execFileSync (node:child_process:926:15)
    at main (/home/erik/src/typescript-runtime-type-benchmarks/index.ts:64:24)
    at Object.<anonymous> (/home/erik/src/typescript-runtime-type-benchmarks/index.ts:101:1)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module.m._compile (/home/erik/src/typescript-runtime-type-benchmarks/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Object.require.extensions.<computed> [as .ts] (/home/erik/src/typescript-runtime-type-benchmarks/node_modules/ts-node/src/index.ts:1621:12) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 41938,
  stdout: null,
  stderr: null
}

}
}
}
break;
Expand Down