Re: [PATCH] test: aggregate-results updates

2021-08-03 Thread David Bremner
Tomi Ollila  writes:

> On Mon, Aug 02 2021, David Bremner wrote:
>
>> Tomi Ollila  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
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH] test: aggregate-results updates

2021-08-02 Thread David Bremner
Tomi Ollila  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?
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH] test: aggregate-results updates

2021-06-07 Thread David Bremner
Tomi Ollila  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
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH] test: aggregate-results updates

2021-06-07 Thread Tomi Ollila
On Sat, Jun 05 2021, David Bremner wrote:

> Tomi Ollila  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-le...@notmuchmail.org
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH] test: aggregate-results updates

2021-06-05 Thread David Bremner
Tomi Ollila  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.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH] test: aggregate-results updates

2021-05-17 Thread 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,