Fix vps and makeself - they now pass CI + syntax_analysis fix #47
Conversation
… visualization additions
There was a problem hiding this comment.
-
I don't think this script should go through the
makefileindirection -- just execute the script that the makefile runs directly. Also, be sure to run it with the environment variable hook for the benchmarks -- refer to e.g.unix50'srun.sh. -
All this setup for
shunit2should be done indeps.sh, not here, unless I'm mistaken.
There was a problem hiding this comment.
This script shouldn't do the same thing as run.sh -- it shouldn't run the benchmark. Instead, it should inspect the outputs produced by run.sh to determine if it ran correctly. See unix50's verify.sh for example
makeself/makeself/test/run-tests.sh
Outdated
| for test in ./*test; | ||
| do | ||
| echo "::group::$test" | ||
| bash "./$test" || { echo "*** ERROR: Test '$test' failed!"; exit 1; } |
There was a problem hiding this comment.
We may want to modify this so that it uses $BENCHMARK_SHELL instead of bash?
makeself/makeself/.err
Outdated
There was a problem hiding this comment.
Unless I'm mistaken, I don't think we want these output files (this, .hash, .out, and release/) committed?
vps-audit/run.sh
Outdated
|
|
||
| # run the audit script and capture real-time output | ||
| echo "Starting VPS audit..." | ||
| "${main_script}" 2>&1 | tee "${log_file}" |
There was a problem hiding this comment.
Also need to use $BENCHMARK_SHELL here
There was a problem hiding this comment.
Could we do better than this by just filtering out the parts of the log that we expect to change (like current memory usage, or time) and then hashing the remaining content?
vps-audit/verify.sh
Outdated
| RESULT_MARKERS=("[PASS]" "[WARN]" "[FAIL]") | ||
| RESULTS_COUNT=$(grep -Eo "$(IFS=\|; echo "${RESULT_MARKERS[*]}")" "$REPORT_FILE" | wc -l) | ||
|
|
||
| [[ "$RESULTS_COUNT" -gt 0 ]] || fail "No result markers ([PASS], [WARN], [FAIL]) found in the report." |
There was a problem hiding this comment.
Otherwise I think we should do better than this: specify the exact sequence of result markers we expect, perhaps the associated messages
infrastructure/syntax_analysis.py
Outdated
There was a problem hiding this comment.
Let's drop this from this PR since we already have #44
Signed-off-by: Evangelos Lamprou <[email protected]>
|
@vagos I believe this is good to merge now. Thank you! |
No description provided.