Do not skip tests in run-tests-zkasm.js
#306
Conversation
|
We require contributors/corporates @MCJOHN974 to read our Contributor License Agreement, please check the Individual CLA document/Corporate CLA document |
tools/run-tests-zkasm.js
Outdated
| // Run all zkasm files | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| console.log(chalk.yellow('--> Start running zkasm files')); | ||
| for (const file of files) { | ||
| if (file.includes('ignore')) | ||
| continue; | ||
| await runTest(file, cmPols); | ||
| if (await runTest(file, cmPols) == 1) { | ||
| exit_code = 1; |
There was a problem hiding this comment.
Given that we don't really care about the exact exit code and we don't use it below, maybe we can change the meaning of this variable to make the code easier to read. E.g. we can name it failed_test_count and report, in the end, the number of failed tests or we can just track a boolean value have_failed_test.
| // Run all zkasm files | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| console.log(chalk.yellow('--> Start running zkasm files')); | ||
| for (const file of files) { | ||
| if (file.includes('ignore')) | ||
| continue; | ||
| await runTest(file, cmPols); | ||
| if (await runTest(file, cmPols) == 1) { |
There was a problem hiding this comment.
From this like, it's not really clear what "1" means. We can either document the function runTest to describe which values it returns or rename the function to make it more obvious (e.g. testPasses).
|
We require contributors/corporates @MCJOHN974 to read our Contributor License Agreement, please check the Individual CLA document/Corporate CLA document |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
Currently, if you run
run-tests-zkasm.jswith directory as an input, and one of files in directory fails, it skips all next tests and determining in which file it was failed is not very obvious. In code in this PR all tests are runned anyway and it is easy to understand which test failed.