* [PATCH] test: aggregate-results updates
@ 2021-05-17 8:11 Tomi Ollila
2021-06-05 11:12 ` David Bremner
2021-08-02 11:11 ` David Bremner
0 siblings, 2 replies; 7+ messages in thread
From: Tomi Ollila @ 2021-05-17 8:11 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
notmuch-test will now call aggregate-results.sh with file list
that it compiles based on the test ran, and aggregate-results
will report failure is any of the test files are missing.
With this notmuch-test no longer has to exit in non-parallel
run if some test fail to write its report file -- so it works
as parallel tests in this sense.
Changed test_done() in test-lib.sh write report file in one write(2),
so there is (even) less chance it being partially written. Also,
now it writes 'total' last and aggregate-results.sh expects this
line to exist in all report files for reporting to be successful.
Added 'set -eu' to notmuch-test and modified code to work with
these settings. That makes it harder to get mistakes slipped
into committed code.
---
testing all changed branches is somewhat hard; I did by adding
sleep 20 and file what effective removed created results files;
another change was to change 'total' to 'toxtal' in test_done,
then I run (at least) the following command lines:
make test
NOTMUCH_TESTS='T140-excludes.sh T740-body.sh' make test
NOTMUCH_TESTS='T140-excludes.sh T740-body.sh' NOTMUCH_TEST_SERIALIZE=t make test
NOTMUCH_TESTS=T160-json.sh NOTMUCH_TEST_SERIALIZE=t test/notmuch-test
test/aggregate-results.sh | 22 ++++++++++++++++--
test/notmuch-test | 47 ++++++++++++++++++++++-----------------
test/test-lib.sh | 15 +++++++------
3 files changed, 54 insertions(+), 30 deletions(-)
diff --git a/test/aggregate-results.sh b/test/aggregate-results.sh
index 75400e6e..6845fcf0 100755
--- a/test/aggregate-results.sh
+++ b/test/aggregate-results.sh
@@ -8,9 +8,16 @@ failed=0
broken=0
total=0
all_skipped=0
+rep_failed=0
for file
do
+ if [ ! -f "$file" ]; then
+ echo "'$file' does not exist!"
+ rep_failed=$((rep_failed + 1))
+ continue
+ fi
+ has_total=0
while read type value
do
case $type in
@@ -24,18 +31,23 @@ do
broken=$((broken + value)) ;;
total)
total=$((total + value))
+ has_total=1
if [ "$value" -eq 0 ]; then
all_skipped=$((all_skipped + 1))
fi
esac
done <"$file"
+ if [ "$has_total" -eq 0 ]; then
+ echo "'$file' lacks 'total ...'; results may be inconsistent."
+ failed=$((failed + 1))
+ fi
done
pluralize_s () { [ "$1" -eq 1 ] && s='' || s='s'; }
echo "Notmuch test suite complete."
-if [ "$fixed" -eq 0 ] && [ "$failed" -eq 0 ]; then
+if [ "$fixed" -eq 0 ] && [ "$failed" -eq 0 ] && [ "$rep_failed" -eq 0 ]; then
pluralize_s "$total"
printf "All $total test$s "
if [ "$broken" -eq 0 ]; then
@@ -70,10 +82,16 @@ if [ "$all_skipped" -ne 0 ]; then
echo "All tests in $all_skipped file$s skipped."
fi
+if [ "$rep_failed" -ne 0 ]; then
+ pluralize_s "$rep_failed"
+ echo "$rep_failed test$s failed to report results."
+fi
+
# Note that we currently do not consider skipped tests as failing the
# build.
-if [ "$success" -gt 0 ] && [ "$fixed" -eq 0 ] && [ "$failed" -eq 0 ]
+if [ "$success" -gt 0 ] && [ "$fixed" -eq 0 ] &&
+ [ "$failed" -eq 0 ] && [ "$rep_failed" -eq 0 ]
then
exit 0
else
diff --git a/test/notmuch-test b/test/notmuch-test
index cbd33f93..ce142f7c 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -18,12 +18,14 @@ fi
# Ensure NOTMUCH_SRCDIR and NOTMUCH_BUILDDIR are set.
. $(dirname "$0")/export-dirs.sh || exit 1
+set -eu
+
TESTS=
-for test in $NOTMUCH_TESTS; do
+for test in ${NOTMUCH_TESTS-}; do
TESTS="$TESTS $NOTMUCH_SRCDIR/test/$test"
done
-if [[ -z "$TESTS" ]]; then
+if [ -z "$TESTS" ]; then
TESTS="$NOTMUCH_SRCDIR/test/T[0-9][0-9][0-9]-*.sh"
fi
@@ -44,43 +46,46 @@ else
TEST_TIMEOUT_CMD=""
fi
-trap 'e=$?; kill $!; exit $e' HUP INT TERM
-
META_FAILURE=
+RES=0
# Run the tests
-if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel >/dev/null ; then
+if test -z "${NOTMUCH_TEST_SERIALIZE-}" && command -v parallel >/dev/null ; then
test -t 1 && export COLORS_WITHOUT_TTY=t || :
if parallel --version 2>&1 | grep -q GNU ; then
echo "INFO: running tests with GNU parallel"
- printf '%s\n' $TESTS | $TEST_TIMEOUT_CMD parallel
+ printf '%s\n' $TESTS | $TEST_TIMEOUT_CMD parallel || RES=$?
else
echo "INFO: running tests with moreutils parallel"
- $TEST_TIMEOUT_CMD parallel -- $TESTS
+ $TEST_TIMEOUT_CMD parallel -- $TESTS || RES=$?
fi
- RES=$?
- if [[ $RES != 0 ]]; then
+ if [ $RES != 0 ]; then
META_FAILURE="parallel test suite returned error code $RES"
fi
else
+ trap 'e=$?; trap - 0; kill ${!-}; exit $e' 0 HUP INT TERM
for test in $TESTS; do
$TEST_TIMEOUT_CMD $test "$@" &
- wait $!
- # If the test failed without producing results, then it aborted,
- # so we should abort, too.
- RES=$?
- testname=$(basename $test .sh)
- if [[ $RES != 0 && ! -e "$NOTMUCH_BUILDDIR/test/test-results/$testname" ]]; then
- META_FAILURE="Aborting on $testname (returned $RES)"
- break
- fi
+ wait $! && ev=0 || ev=$?
+ test $ev = 0 || RES=$ev
done
+ trap - 0 HUP INT TERM
+ if [ $RES != 0 ]; then
+ META_FAILURE="some tests failed; first failed returned error code $RES"
+ fi
fi
-trap - HUP INT TERM
# Report results
+RESULT_FILES=
+for file in $TESTS
+do
+ file=${file##*/} # drop leading path components
+ file=${file%.sh} # drop trailing '.sh'
+ RESULT_FILES="$RESULT_FILES $NOTMUCH_BUILDDIR/test/test-results/$file"
+done
+
echo
-$NOTMUCH_SRCDIR/test/aggregate-results.sh $NOTMUCH_BUILDDIR/test/test-results/*
-ev=$?
+$NOTMUCH_SRCDIR/test/aggregate-results.sh $RESULT_FILES && ev=0 || ev=$?
+
if [ -n "$META_FAILURE" ]; then
printf 'ERROR: %s\n' "$META_FAILURE"
if [ $ev = 0 ]; then
diff --git a/test/test-lib.sh b/test/test-lib.sh
index d7c96a93..2dceb497 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1021,15 +1021,16 @@ test_when_finished () {
test_done () {
GIT_EXIT_OK=t
test_results_dir="$TEST_DIRECTORY/test-results"
- mkdir -p "$test_results_dir"
+ test -d "$test_results_dir" || mkdir "$test_results_dir"
test_results_path="$test_results_dir/$this_test"
- echo "total $test_count" >> $test_results_path
- echo "success $test_success" >> $test_results_path
- echo "fixed $test_fixed" >> $test_results_path
- echo "broken $test_broken" >> $test_results_path
- echo "failed $test_failure" >> $test_results_path
- echo "" >> $test_results_path
+ printf %s\\n \
+ "success $test_success" \
+ "fixed $test_fixed" \
+ "broken $test_broken" \
+ "failed $test_failure" \
+ "total $test_count" \
+ > $test_results_path
[ -n "$EMACS_SERVER" ] && test_emacs '(kill-emacs)'
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] test: aggregate-results updates
2021-05-17 8:11 [PATCH] test: aggregate-results updates Tomi Ollila
@ 2021-06-05 11:12 ` David Bremner
2021-06-07 15:13 ` Tomi Ollila
2021-08-02 11:11 ` David Bremner
1 sibling, 1 reply; 7+ messages in thread
From: David Bremner @ 2021-06-05 11:12 UTC (permalink / raw)
To: Tomi Ollila, notmuch; +Cc: tomi.ollila
Tomi Ollila <tomi.ollila@iki.fi> writes:
>
> testing all changed branches is somewhat hard; I did by adding
> sleep 20 and file what effective removed created results files;
> another change was to change 'total' to 'toxtal' in test_done,
> then I run (at least) the following command lines:
changing test_done results in pretty nonsensical output. I guess that's
OK?
$ NOTMUCH_TESTS=T310-emacs.sh ./notmuch-test
...
'/home/bremner/software/upstream/notmuch/test/test-results/T310-emacs' lacks 'total ...'; results may be inconsistent.
Notmuch test suite complete.
72/0 tests passed.
1 test failed.
-73 tests skipped.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] test: aggregate-results updates
2021-06-05 11:12 ` David Bremner
@ 2021-06-07 15:13 ` Tomi Ollila
2021-06-07 23:35 ` David Bremner
0 siblings, 1 reply; 7+ messages in thread
From: Tomi Ollila @ 2021-06-07 15:13 UTC (permalink / raw)
To: David Bremner, notmuch
On Sat, Jun 05 2021, David Bremner wrote:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
>>
>> testing all changed branches is somewhat hard; I did by adding
>> sleep 20 and file what effective removed created results files;
>> another change was to change 'total' to 'toxtal' in test_done,
>> then I run (at least) the following command lines:
>
> changing test_done results in pretty nonsensical output. I guess that's
> OK?
Yes.
It is very unlikely that if something is written to file not all data were
written (as the amount of data to be written is low).
A bit less unlikely is the case that output file is created but no data
written.
Should such an extremely rare cases happen it should be enough just to
report it so it get noticed (and not add any extra code to handle such
cases any further). One checking the results would probably guess that
if output is as nonsensical as it is something went badly wrong.
Tomi
>
> $ NOTMUCH_TESTS=T310-emacs.sh ./notmuch-test
>
> ...
>
> '/home/bremner/software/upstream/notmuch/test/test-results/T310-emacs' lacks 'total ...'; results may be inconsistent.
> Notmuch test suite complete.
> 72/0 tests passed.
> 1 test failed.
> -73 tests skipped.
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] test: aggregate-results updates
2021-06-07 15:13 ` Tomi Ollila
@ 2021-06-07 23:35 ` David Bremner
0 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2021-06-07 23:35 UTC (permalink / raw)
To: Tomi Ollila, notmuch
Tomi Ollila <tomi.ollila@iki.fi> writes:
>
> Should such an extremely rare cases happen it should be enough just to
> report it so it get noticed (and not add any extra code to handle such
> cases any further). One checking the results would probably guess that
> if output is as nonsensical as it is something went badly wrong.
>
> Tomi
>
Patch applied to master.
d
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] test: aggregate-results updates
2021-05-17 8:11 [PATCH] test: aggregate-results updates Tomi Ollila
2021-06-05 11:12 ` David Bremner
@ 2021-08-02 11:11 ` David Bremner
2021-08-03 19:39 ` Tomi Ollila
1 sibling, 1 reply; 7+ messages in thread
From: David Bremner @ 2021-08-02 11:11 UTC (permalink / raw)
To: Tomi Ollila, notmuch; +Cc: tomi.ollila
Tomi Ollila <tomi.ollila@iki.fi> writes:
> - mkdir -p "$test_results_dir"
> + test -d "$test_results_dir" || mkdir "$test_results_dir"
Lately I've notice some complaints during parallel test running
mkdir: cannot create directory '/home/bremner/software/upstream/notmuch/test/test-results': File exists
It seems like this change might be the culprit. Can you explain why it
was needed?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] test: aggregate-results updates
2021-08-02 11:11 ` David Bremner
@ 2021-08-03 19:39 ` Tomi Ollila
2021-08-04 1:46 ` David Bremner
0 siblings, 1 reply; 7+ messages in thread
From: Tomi Ollila @ 2021-08-03 19:39 UTC (permalink / raw)
To: David Bremner, notmuch
On Mon, Aug 02 2021, David Bremner wrote:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> - mkdir -p "$test_results_dir"
>> + test -d "$test_results_dir" || mkdir "$test_results_dir"
>
> Lately I've notice some complaints during parallel test running
>
> mkdir: cannot create directory '/home/bremner/software/upstream/notmuch/test/test-results': File exists
>
> It seems like this change might be the culprit. Can you explain why it
> was needed?
ah, parallel test non-atomicity is the problem; fix would be to restore -p to mkdir
(so it does not fail when another program executed mkdir between that test
check and mkdir execution)
i.e.
test -d "$test_results_dir" || mkdir -p "$test_results_dir"
Checking the dir with shell builtin drops need to execute mkdir in external
process, which makes the tests run a bit snappier (in small increments).
Tomi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] test: aggregate-results updates
2021-08-03 19:39 ` Tomi Ollila
@ 2021-08-04 1:46 ` David Bremner
0 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2021-08-04 1:46 UTC (permalink / raw)
To: Tomi Ollila, notmuch
Tomi Ollila <tomi.ollila@iki.fi> writes:
> On Mon, Aug 02 2021, David Bremner wrote:
>
>> Tomi Ollila <tomi.ollila@iki.fi> writes:
>>
>>> - mkdir -p "$test_results_dir"
>>> + test -d "$test_results_dir" || mkdir "$test_results_dir"
>>
>> Lately I've notice some complaints during parallel test running
>>
>> mkdir: cannot create directory '/home/bremner/software/upstream/notmuch/test/test-results': File exists
>>
>> It seems like this change might be the culprit. Can you explain why it
>> was needed?
>
> ah, parallel test non-atomicity is the problem; fix would be to restore -p to mkdir
> (so it does not fail when another program executed mkdir between that test
> check and mkdir execution)
>
Switched back to mkdir -p here, the occasional test failure is worse
than the potential slowdown.
d
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-04 1:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 8:11 [PATCH] test: aggregate-results updates Tomi Ollila
2021-06-05 11:12 ` David Bremner
2021-06-07 15:13 ` Tomi Ollila
2021-06-07 23:35 ` David Bremner
2021-08-02 11:11 ` David Bremner
2021-08-03 19:39 ` Tomi Ollila
2021-08-04 1:46 ` David Bremner
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).