From 960584c0f3e8d0e9909b5b98a057dc0faa3f22ff Mon Sep 17 00:00:00 2001 From: David Given Date: Tue, 29 Nov 2016 20:59:43 +0100 Subject: [PATCH 1/8] Replace the hacky and broken pipeline in testdriver.sh with a custom-written tool in C; much more robust and easier to understand, as well as avoiding the dependency on timeout (which isn't Posix). --- tests/plat/build.lua | 5 +- tests/plat/testdriver.sh | 65 ++++++++++++------------ util/build/build.lua | 7 +++ util/build/testrunner.c | 104 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 35 deletions(-) create mode 100644 util/build/build.lua create mode 100644 util/build/testrunner.c diff --git a/tests/plat/build.lua b/tests/plat/build.lua index 3873ed9a5..be65668fd 100644 --- a/tests/plat/build.lua +++ b/tests/plat/build.lua @@ -45,10 +45,11 @@ definerule("plat_testsuite", outleaves = { "stamp" }, ins = { bin, - "tests/plat/testdriver.sh" + "tests/plat/testdriver.sh", + "util/build+testrunner" }, commands = { - "%{ins[2]} "..e.method.." %{ins[1]} 5", + "%{ins[2]} "..e.method.." %{ins[1]} 5 %{ins[3]}", "touch %{outs}" } } diff --git a/tests/plat/testdriver.sh b/tests/plat/testdriver.sh index 272c6b2b7..6af14da0f 100755 --- a/tests/plat/testdriver.sh +++ b/tests/plat/testdriver.sh @@ -2,50 +2,49 @@ method=$1 img=$2 timeout=$3 +timeoutprog=$4 -pipe=/tmp/$$.testdriver.pipe -mknod $pipe p -trap "rm -f $pipe" EXIT +set -e result=/tmp/$$.testdriver.result trap "rm -f $result" EXIT -pidfile=/tmp/$$.testdriver.pid -trap "rm -f $pidfile" EXIT +errcho() { + >&2 echo "$*" +} -case $method in - qemu-system-*) - if ! command -v $method >/dev/null 2>&1 ; then - echo "Warning: $method not installed, skipping test" - exit 0 - fi +get_test_output() { + case $method in + qemu-system-*) + if ! command -v $method >/dev/null 2>&1 ; then + errcho "Warning: $method not installed, skipping test" + exit 0 + fi - case $method in - qemu-system-i386) img="-drive file=$img,if=floppy,format=raw" ;; - qemu-system-ppc) img="-kernel $img" ;; - esac + case $method in + qemu-system-i386) img="-drive file=$img,if=floppy,format=raw" ;; + qemu-system-ppc) img="-kernel $img" ;; + esac - ( $method -nographic $img 2>&1 & echo $! > $pidfile ) \ - | tee $result \ - | ( timeout $timeout grep -l -q @@FINISHED ; echo ) \ - | ( read dummy && kill $(cat $pidfile) ) - - ;; + $timeoutprog -t $timeout -- $method -nographic $img > $result + ;; - qemu-*) - if ! command -v $method >/dev/null 2>&1 ; then - echo "Warning: $method not installed, skipping test" - exit 0 - fi + qemu-*) + if ! command -v $method >/dev/null 2>&1 ; then + errcho "Warning: $method not installed, skipping test" + exit 0 + fi - $method $img > $result - ;; + $method $img > $result + ;; - *) - echo "Error: $method not known by testdriver" - exit 1 - ;; -esac + *) + errcho "Error: $method not known by testdriver" + exit 1 + ;; + esac +} +get_test_output > $result ( grep -q @@FAIL $result || ! grep -q @@FINISHED $result ) && cat $result && exit 1 exit 0 diff --git a/util/build/build.lua b/util/build/build.lua new file mode 100644 index 000000000..76623fe91 --- /dev/null +++ b/util/build/build.lua @@ -0,0 +1,7 @@ +cprogram { + name = "testrunner", + srcs = { "./testrunner.c" }, + deps = { + "modules/src/data+lib" + } +} diff --git a/util/build/testrunner.c b/util/build/testrunner.c new file mode 100644 index 000000000..16d526d9a --- /dev/null +++ b/util/build/testrunner.c @@ -0,0 +1,104 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include "diagnostics.h" + +static bool timed_out = false; +static bool child_exited = false; +static char** command = NULL; +static int timeout = 0; +static jmp_buf exit_jmp; + +static void parse_arguments(int argc, char* const argv[]) +{ + program_name = argv[0]; + for (;;) + { + int c = getopt(argc, argv, "t:"); + if (c == -1) + break; + + switch (c) + { + case 't': + timeout = atoi(optarg); + break; + + default: + fatal("syntax: testrunner -- \n"); + } + } + + command = &argv[optind]; + if (!command[0]) + fatal("you must supply a command"); + if (timeout <= 0) + fatal("timeout missing or invalid"); +} + +static void sigalrm_cb(int sigraised) +{ + timed_out = true; + longjmp(exit_jmp, 1); +} + +int main(int argc, char* const argv[]) +{ + const int READ = 0; + const int WRITE = 1; + + int fds[2]; + int pid; + FILE* childin; + int wstatus; + char buffer[4096]; + + parse_arguments(argc, argv); + + if (setjmp(exit_jmp) == 0) + { + /* First time through. */ + + signal(SIGALRM, sigalrm_cb); + alarm(timeout); + + pipe(fds); + pid = fork(); + if (pid == 0) + { + /* Child */ + close(fds[READ]); + close(0); + dup2(fds[WRITE], 1); + dup2(fds[WRITE], 2); + execvp(command[0], command); + _exit(1); + } + + childin = fdopen(fds[READ], "r"); + while (!timed_out) + { + if (!fgets(buffer, sizeof(buffer), childin)) + break; + fputs(buffer, stdout); + + if (strcmp(buffer, "@@FINISHED\n") == 0) + break; + if (strcmp(buffer, "@@FINISHED\r\n") == 0) + break; + } + } + + /* Reached via longjmp, EOF, or seeing a @@FINISHED. */ + + kill(pid, SIGKILL); + waitpid(pid, &wstatus, 0); + if (timed_out) + exit(1); + exit(WEXITSTATUS(wstatus)); +} From b7de58e34ebb6150974fca24c6e07e4b46d21b0d Mon Sep 17 00:00:00 2001 From: David Given Date: Thu, 1 Dec 2016 22:05:22 +0100 Subject: [PATCH 2/8] Fix signal unsafety in testdriver.c. --- util/build/testrunner.c | 72 ++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/util/build/testrunner.c b/util/build/testrunner.c index 16d526d9a..e666ec4ac 100644 --- a/util/build/testrunner.c +++ b/util/build/testrunner.c @@ -10,9 +10,9 @@ static bool timed_out = false; static bool child_exited = false; -static char** command = NULL; +static char* const* command = NULL; static int timeout = 0; -static jmp_buf exit_jmp; +static int pid = 0; static void parse_arguments(int argc, char* const argv[]) { @@ -44,7 +44,8 @@ static void parse_arguments(int argc, char* const argv[]) static void sigalrm_cb(int sigraised) { timed_out = true; - longjmp(exit_jmp, 1); + if (pid) + kill(pid, SIGKILL); } int main(int argc, char* const argv[]) @@ -53,48 +54,51 @@ int main(int argc, char* const argv[]) const int WRITE = 1; int fds[2]; - int pid; FILE* childin; int wstatus; char buffer[4096]; parse_arguments(argc, argv); - if (setjmp(exit_jmp) == 0) + pipe(fds); + pid = fork(); + if (pid == 0) { - /* First time through. */ - + /* Child */ + close(fds[READ]); + close(0); + dup2(fds[WRITE], 1); + dup2(fds[WRITE], 2); + execvp(command[0], command); + _exit(1); + } + else + { + /* Parent */ + close(fds[WRITE]); signal(SIGALRM, sigalrm_cb); alarm(timeout); - - pipe(fds); - pid = fork(); - if (pid == 0) - { - /* Child */ - close(fds[READ]); - close(0); - dup2(fds[WRITE], 1); - dup2(fds[WRITE], 2); - execvp(command[0], command); - _exit(1); - } - - childin = fdopen(fds[READ], "r"); - while (!timed_out) - { - if (!fgets(buffer, sizeof(buffer), childin)) - break; - fputs(buffer, stdout); - - if (strcmp(buffer, "@@FINISHED\n") == 0) - break; - if (strcmp(buffer, "@@FINISHED\r\n") == 0) - break; - } } - /* Reached via longjmp, EOF, or seeing a @@FINISHED. */ + childin = fdopen(fds[READ], "r"); + if (!childin) + fatal("cannot open pipe"); + + while (!timed_out) + { + if (!fgets(buffer, sizeof(buffer), childin)) + break; + fputs(buffer, stdout); + + if (strcmp(buffer, "@@FINISHED\n") == 0) + break; + if (strcmp(buffer, "@@FINISHED\r\n") == 0) + break; + } + + /* Reached via EOF or seeing a @@FINISHED. */ + + alarm(0); kill(pid, SIGKILL); waitpid(pid, &wstatus, 0); From 8c99e2b7ad7647197f9f0f96e82f8d777dc0edcc Mon Sep 17 00:00:00 2001 From: David Given Date: Thu, 1 Dec 2016 23:03:30 +0100 Subject: [PATCH 3/8] Run all tests, even the ones which fail, and emit a test summary right at the end of the build (and fail then). --- Makefile | 2 +- build.lua | 16 +++++++++++++--- first/testsummary.sh | 11 +++++++++++ tests/plat/_dummy.c | 1 - tests/plat/build.lua | 11 ++++------- tests/plat/lib/test.c | 7 +++++-- tests/plat/testdriver.sh | 6 +++--- util/build/build.lua | 1 + util/build/testrunner.c | 13 +++++++++++-- 9 files changed, 49 insertions(+), 19 deletions(-) create mode 100755 first/testsummary.sh diff --git a/Makefile b/Makefile index febd87aff..4399f0c28 100644 --- a/Makefile +++ b/Makefile @@ -63,7 +63,7 @@ PLATDEP = $(INSDIR)/lib/ack .NOTPARALLEL: -MAKECMDGOALS ?= +ack +MAKECMDGOALS ?= +ack +tests BUILD_FILES = $(shell find * -name '*.lua') ifneq ($(shell which ninja),) diff --git a/build.lua b/build.lua index 607a83a3b..98ee9ff9b 100644 --- a/build.lua +++ b/build.lua @@ -46,8 +46,18 @@ installable { "examples+pkg", plat_packages }, - deps = { - test_packages - } } +normalrule { + name = "tests", + ins = { + "first/testsummary.sh", + test_packages + }, + outleaves = { + "stamp" + }, + commands = { + "%{ins}" + } +} diff --git a/first/testsummary.sh b/first/testsummary.sh new file mode 100755 index 000000000..4ad46d52d --- /dev/null +++ b/first/testsummary.sh @@ -0,0 +1,11 @@ +#!/bin/sh +failed=$(find "$@" ! -size 0) +echo "" +echo "$(echo $failed | wc -w) failed tests" +echo "" +for a in $failed; do + echo "**** $a" + cat $a + echo "" +done +exec test "$failed" == "" diff --git a/tests/plat/_dummy.c b/tests/plat/_dummy.c index a4693300f..48104b5aa 100644 --- a/tests/plat/_dummy.c +++ b/tests/plat/_dummy.c @@ -6,4 +6,3 @@ void _m_a_i_n(void) ASSERT(0 == 0); finished(); } - diff --git a/tests/plat/build.lua b/tests/plat/build.lua index be65668fd..f5885a190 100644 --- a/tests/plat/build.lua +++ b/tests/plat/build.lua @@ -42,24 +42,21 @@ definerule("plat_testsuite", tests[#tests+1] = normalrule { name = fs, - outleaves = { "stamp" }, + outleaves = { e.plat.."-"..fs.."-testlog.txt" }, ins = { bin, "tests/plat/testdriver.sh", "util/build+testrunner" }, commands = { - "%{ins[2]} "..e.method.." %{ins[1]} 5 %{ins[3]}", - "touch %{outs}" + "(%{ins[2]} "..e.method.." %{ins[1]} 5 %{ins[3]} || echo FAILED) 2>&1 > %{outs}", } } end - return normalrule { + return bundle { name = e.name, - outleaves = { "stamp" }, - ins = tests, - commands = { "touch %{outs}" } + srcs = tests, } end ) \ No newline at end of file diff --git a/tests/plat/lib/test.c b/tests/plat/lib/test.c index df00e1089..426f9944a 100644 --- a/tests/plat/lib/test.c +++ b/tests/plat/lib/test.c @@ -26,7 +26,10 @@ void writehex(uint32_t code) void fail(uint32_t code) { - write(1, "@@FAIL 0x", 10); + static const char fail_msg[] = "@@FAIL 0x"; + static const char nl_msg[] = "\n"; + + write(1, fail_msg, sizeof(fail_msg)-1); writehex(code); - write(1, "\n", 1); + write(1, nl_msg, sizeof(nl_msg)-1); } diff --git a/tests/plat/testdriver.sh b/tests/plat/testdriver.sh index 6af14da0f..6c421f798 100755 --- a/tests/plat/testdriver.sh +++ b/tests/plat/testdriver.sh @@ -26,7 +26,7 @@ get_test_output() { qemu-system-ppc) img="-kernel $img" ;; esac - $timeoutprog -t $timeout -- $method -nographic $img > $result + $timeoutprog -t $timeout -- $method -nographic $img 2>&1 > $result ;; qemu-*) @@ -35,7 +35,7 @@ get_test_output() { exit 0 fi - $method $img > $result + $method $img 2>&1 > $result ;; *) @@ -45,6 +45,6 @@ get_test_output() { esac } -get_test_output > $result +get_test_output ( grep -q @@FAIL $result || ! grep -q @@FINISHED $result ) && cat $result && exit 1 exit 0 diff --git a/util/build/build.lua b/util/build/build.lua index 76623fe91..41bd0b6ad 100644 --- a/util/build/build.lua +++ b/util/build/build.lua @@ -5,3 +5,4 @@ cprogram { "modules/src/data+lib" } } + diff --git a/util/build/testrunner.c b/util/build/testrunner.c index e666ec4ac..fb2add562 100644 --- a/util/build/testrunner.c +++ b/util/build/testrunner.c @@ -6,6 +6,7 @@ #include #include #include +#include #include "diagnostics.h" static bool timed_out = false; @@ -57,6 +58,7 @@ int main(int argc, char* const argv[]) FILE* childin; int wstatus; char buffer[4096]; + char* p; parse_arguments(argc, argv); @@ -90,9 +92,13 @@ int main(int argc, char* const argv[]) break; fputs(buffer, stdout); - if (strcmp(buffer, "@@FINISHED\n") == 0) + p = buffer; + while (isspace(*p)) + p++; + + if (strcmp(p, "@@FINISHED\n") == 0) break; - if (strcmp(buffer, "@@FINISHED\r\n") == 0) + if (strcmp(p, "@@FINISHED\r\n") == 0) break; } @@ -103,6 +109,9 @@ int main(int argc, char* const argv[]) kill(pid, SIGKILL); waitpid(pid, &wstatus, 0); if (timed_out) + { + fprintf(stderr, "@@TIMEDOUT\n"); exit(1); + } exit(WEXITSTATUS(wstatus)); } From 9e6581b0ff4505ff62cf4aafa536ddf3d63f490c Mon Sep 17 00:00:00 2001 From: David Given Date: Thu, 1 Dec 2016 23:14:29 +0100 Subject: [PATCH 4/8] Mark skipped tests in the logs (so we can get stats on them later). --- tests/plat/testdriver.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/plat/testdriver.sh b/tests/plat/testdriver.sh index 6c421f798..a0fed5686 100755 --- a/tests/plat/testdriver.sh +++ b/tests/plat/testdriver.sh @@ -18,6 +18,7 @@ get_test_output() { qemu-system-*) if ! command -v $method >/dev/null 2>&1 ; then errcho "Warning: $method not installed, skipping test" + echo "@@SKIPPED" > $result exit 0 fi @@ -32,6 +33,7 @@ get_test_output() { qemu-*) if ! command -v $method >/dev/null 2>&1 ; then errcho "Warning: $method not installed, skipping test" + echo "@@SKIPPED" > $result exit 0 fi @@ -46,5 +48,5 @@ get_test_output() { } get_test_output -( grep -q @@FAIL $result || ! grep -q @@FINISHED $result ) && cat $result && exit 1 +( grep -q '@@FAIL\|@@SKIPPED' $result || ! grep -q @@FINISHED $result ) && cat $result && exit 1 exit 0 From 467709ff38ac08be7b5bf127fabd32da26a95855 Mon Sep 17 00:00:00 2001 From: David Given Date: Fri, 2 Dec 2016 00:00:31 +0100 Subject: [PATCH 5/8] Report skipped, failed and timed out tests. --- first/testsummary.sh | 18 ++++++++++++++---- tests/plat/testdriver.sh | 4 ++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/first/testsummary.sh b/first/testsummary.sh index 4ad46d52d..be0842afa 100755 --- a/first/testsummary.sh +++ b/first/testsummary.sh @@ -1,11 +1,21 @@ #!/bin/sh -failed=$(find "$@" ! -size 0) +notsucceeding=$(find "$@" ! -size 0) echo "" -echo "$(echo $failed | wc -w) failed tests" +echo "$(echo $notsucceeding | wc -w) tests failed to pass" + +skipped=$(grep -l @@SKIPPED $notsucceeding) +echo "$(echo $skipped | wc -w) were skipped (see build log for details)" + +timedout=$(grep -l @@TIMEDOUT $notsucceeding) +echo "$(echo $timedout | wc -w) timed out" + +failed=$(grep -l @@FAIL $notsucceeding) +echo "$(echo $failed | wc -w) failed" + echo "" -for a in $failed; do +for a in $failed $timedout; do echo "**** $a" cat $a echo "" done -exec test "$failed" == "" +exec test "$failed" == "" -o "$timedout" == "" diff --git a/tests/plat/testdriver.sh b/tests/plat/testdriver.sh index a0fed5686..5e8e5b899 100755 --- a/tests/plat/testdriver.sh +++ b/tests/plat/testdriver.sh @@ -18,7 +18,7 @@ get_test_output() { qemu-system-*) if ! command -v $method >/dev/null 2>&1 ; then errcho "Warning: $method not installed, skipping test" - echo "@@SKIPPED" > $result + echo "@@SKIPPED" exit 0 fi @@ -33,7 +33,7 @@ get_test_output() { qemu-*) if ! command -v $method >/dev/null 2>&1 ; then errcho "Warning: $method not installed, skipping test" - echo "@@SKIPPED" > $result + echo "@@SKIPPED" exit 0 fi From 3aa048728983a33c4cc46871c093d6276af87d8d Mon Sep 17 00:00:00 2001 From: David Given Date: Fri, 2 Dec 2016 00:10:33 +0100 Subject: [PATCH 6/8] UI tweaks to the test summary. --- first/testsummary.sh | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/first/testsummary.sh b/first/testsummary.sh index be0842afa..9f1002b39 100755 --- a/first/testsummary.sh +++ b/first/testsummary.sh @@ -1,21 +1,33 @@ #!/bin/sh -notsucceeding=$(find "$@" ! -size 0) echo "" -echo "$(echo $notsucceeding | wc -w) tests failed to pass" -skipped=$(grep -l @@SKIPPED $notsucceeding) -echo "$(echo $skipped | wc -w) were skipped (see build log for details)" +succeeding="$(find "$@" -size 0)" +notsucceeding="$(find "$@" ! -size 0)" +skipped="$(grep -l @@SKIPPED $notsucceeding)" +timedout="$(grep -l @@TIMEDOUT $notsucceeding)" +failed="$(grep -l @@FAIL $notsucceeding)" -timedout=$(grep -l @@TIMEDOUT $notsucceeding) -echo "$(echo $timedout | wc -w) timed out" - -failed=$(grep -l @@FAIL $notsucceeding) -echo "$(echo $failed | wc -w) failed" - -echo "" for a in $failed $timedout; do echo "**** $a" cat $a echo "" done -exec test "$failed" == "" -o "$timedout" == "" + +echo "$(echo $succeeding | wc -w) tests passed" +echo "$(echo $notsucceeding | wc -w) tests failed to pass" +echo "$(echo $skipped | wc -w) were skipped (see build log for details)" +echo "$(echo $timedout | wc -w) timed out" +echo "$(echo $failed | wc -w) failed" +echo "" + +if [ "$failed" != "" -o "$timedout" != "" ]; then + echo "Test status: SAD FACE (tests are failing)" + exit 1 +fi +if [ "$succeeding" = "" ]; then + echo "Test status: PUZZLED FACE (all tests were skipped)" + exit 1 +fi +echo "Test status: HAPPY FACE (all tests are passing)" +exit 0 + From 213def96664ba7bb1e0a3e72b8a9439f9038daed Mon Sep 17 00:00:00 2001 From: David Given Date: Fri, 2 Dec 2016 00:14:40 +0100 Subject: [PATCH 7/8] Don't fail to build if all tests are skipped. --- first/testsummary.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/first/testsummary.sh b/first/testsummary.sh index 9f1002b39..4923b0787 100755 --- a/first/testsummary.sh +++ b/first/testsummary.sh @@ -26,7 +26,11 @@ if [ "$failed" != "" -o "$timedout" != "" ]; then fi if [ "$succeeding" = "" ]; then echo "Test status: PUZZLED FACE (all tests were skipped)" - exit 1 + exit 0 +fi +if [ "$skipped" != "" ]; then + echo "Test status: MILDLY PLEASED FACE (some tests were skipped, but the rest pass)" + echo 0 fi echo "Test status: HAPPY FACE (all tests are passing)" exit 0 From 353e2d284270a54ae01d44819cb461e77e023e0d Mon Sep 17 00:00:00 2001 From: David Given Date: Fri, 2 Dec 2016 00:18:44 +0100 Subject: [PATCH 8/8] Typo fix. --- first/testsummary.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/first/testsummary.sh b/first/testsummary.sh index 4923b0787..6301cb977 100755 --- a/first/testsummary.sh +++ b/first/testsummary.sh @@ -30,7 +30,7 @@ if [ "$succeeding" = "" ]; then fi if [ "$skipped" != "" ]; then echo "Test status: MILDLY PLEASED FACE (some tests were skipped, but the rest pass)" - echo 0 + exit 0 fi echo "Test status: HAPPY FACE (all tests are passing)" exit 0