evaluate2.sh: Check output of warmup run and abort early if failed #333
Conversation
| function print_and_execute() { | ||
| echo "+ $@" >&2 | ||
| "$@" | ||
| } |
There was a problem hiding this comment.
Some of the changes in this PR are refactoring to use this method instead of xtrace or echo statements
hundredwatt
force-pushed
the
evaluate2-tests
branch
from
8a8efc6
to
7f009de
Compare
|
I pushed changes based on @AlexanderYastrebov's review. I had some questions previously, but I closed them and ended up resolving the issues (I believe) so that you two can re-review and merge this before I wake up if it looks good to you 😄 My resolution was: instead of silencing This way, it's no longer noisy in the happy path but we still see the diffs printed on test failures :) I replaced the Happy Path
(sending the output from test.sh's invocation of prepare_$fork.sh to /dev/null still didn't silence sdkman's output even if I added On Error
|
hundredwatt
force-pushed
the
evaluate2-tests
branch
6 times, most recently
from
7c5c3ed
to
cdd2117
Compare
…sh also invokes it
This reverts commit 13e9fb7.
hundredwatt
force-pushed
the
evaluate2-tests
branch
from
cdd2117
to
f609760
Compare
|
@AlexanderYastrebov @gunnarmorling I removed the 
|
|
Added colors 😄 
|
Nice. On that diff, any chance we could get word diff? That's why I used |
|
@gunnarmorling git word diff has some issues working with process substitution, see #333 (comment) IMO the diff from #333 (comment) is quite clear (and shows the problem #49) |
I'm not so sure :) How would you find that difference in the 10K test case? Some for the expected output of the challenge. That's where word diff helps a lot. |
|
Ah, hold on. This is diffing now one station per line due to |
Yes... so this should be ready to merge 👍 |
|
Tested this some more, all looking good. Merging now. Thanks a lot, @hundredwatt! It's a very nice improvement, all in one go now, sweet! |
|
I'm also gonna delete the old evaluate script, it's not needed any more. |
|
@hundredwatt, so after some more consideration, can we bring back the logging of the test output? I.e. the "Validating..." lines, and also the output of Graal VM native binary builds. Right now, one is flying a bit blind in regards to progress, in particular when the latter takes a few seconds longer. |
|
@gunnarmorling Sure: #377 |
…unnarmorling#333) * refactor: replace xtrace with "print_and_execute" function * nit: stylize error messages * replace out_expected.txt with measurements_1B.out * print * prevent errors on cleanup * run tests and check warmup run output before running benchmark * move "git diff" pretty diff output to test.sh * Ensure "set -e" is re-enabled if we followed a "continue" branch * add timeouts to test.sh invocations * use diff with tocsv.sh to show differences on failed test * add --quiet mode to test.sh * move prepare_$fork.sh invocation to right below hyperfine since test.sh also invokes it * Revert "add --quiet mode to test.sh" This reverts commit 13e9fb7. * use tee to capture test output to a temp file and print contents on failure --------- Co-authored-by: Jason Nochlin <[email protected]>
Closes #313
out_expected.txtto createmeasurements_1B.out./test.shbefore hyperfine./test.sh <fork> measurements_1B.txtand compare its result to measurements_1B.outNew Output
In screenshot below:
hundredwattfork passes testsall_badfails to pass for all testsbad_1bpasses test.sh default suite, but fails on test fileNOTE: for development purposes, I used a 100m row file and only 3 runs. This is reflected in the screenshot below, but not in this PR's code.