unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] test: aggregate-results updates
@ 2021-05-17  8:11 Tomi Ollila
  2021-06-05 11:12 ` David Bremner
  0 siblings, 1 reply; 4+ 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	[flat|nested] 4+ 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
  0 siblings, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread

end of thread, other threads:[~2021-06-07 23:35 UTC | newest]

Thread overview: 4+ 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

unofficial mirror of notmuch@notmuchmail.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/notmuch/0 notmuch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 notmuch notmuch/ https://yhetil.org/notmuch \
		notmuch@notmuchmail.org
	public-inbox-index notmuch

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.mail.notmuch.general
	nntp://news.gmane.io/gmane.mail.notmuch.general


code repositories for project(s) associated with this inbox:

	notmuch.git.git (no URL configured)

AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git