From bf7ae6fb25a4722886880f5d23de671bd4dbcecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jun 2020 11:19:09 +0200 Subject: [PATCH 1/7] Silence dd invocation in all.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It brings no value and distracts us from the actual content. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 0a9d8063f..06d9c5c35 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1886,7 +1886,7 @@ run_component () { # Unconditionally create a seedfile that's sufficiently long. # Do this before each component, because a previous component may # have messed it up or shortened it. - dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 + dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 >/dev/null 2>&1 # Run the component code. "$@" From 2b2bdaa793d42d58f7048d46859224041d4ea6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jun 2020 11:28:07 +0200 Subject: [PATCH 2/7] Add a --quiet option to all.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The primary purpose is to use it to run all.sh -k -q in the pre-push hook, but this can be useful in any circumstance where you're not interested in the full output from each component and just want a short summary of which components were run (and if any failed). Note that only stdout from components is suppressed, stderr is preserved so that errors are reported. This means components should avoid printing to stderr in normal usage (ie in the absence of errors). Currently all the `check_*` components obey this convention except: - check_generate_test_code: unittest prints progress to stderr - check_test_cases: lots of non-fatal warnings printed to stderr These components will be fixed in follow-up commits. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 06d9c5c35..fb9e50f99 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -120,6 +120,7 @@ pre_initialize_variables () { append_outcome=0 MEMORY=0 FORCE=0 + QUIET=0 KEEP_GOING=0 : ${MBEDTLS_TEST_OUTCOME_FILE=} @@ -200,6 +201,7 @@ Special options: --list-components List components supported on this platform and exit. General options: + -q|--quiet Only output component names, and errors if any. -f|--force Force the tests to overwrite any modified files. -k|--keep-going Run all tests and report errors at the end. -m|--memory Additional optional memory tests. @@ -215,6 +217,7 @@ General options: --no-force Refuse to overwrite modified files (default). --no-keep-going Stop at the first error (default). --no-memory No additional memory tests (default). + --no-quiet Print full ouput from components. --out-of-source-dir= Directory used for CMake out-of-source build tests. --outcome-file= File where test outcomes are written (not done if empty; default: \$MBEDTLS_TEST_OUTCOME_FILE). @@ -288,6 +291,11 @@ msg() else current_section="$1" fi + + if [ $QUIET -eq 1 ]; then + return + fi + echo "" echo "******************************************************************" echo "* $current_section " @@ -363,11 +371,13 @@ pre_parse_command_line () { --no-force) FORCE=0;; --no-keep-going) KEEP_GOING=0;; --no-memory) MEMORY=0;; + --no-quiet) QUIET=0;; --openssl) shift; OPENSSL="$1";; --openssl-legacy) shift; OPENSSL_LEGACY="$1";; --openssl-next) shift; OPENSSL_NEXT="$1";; --outcome-file) shift; MBEDTLS_TEST_OUTCOME_FILE="$1";; --out-of-source-dir) shift; OUT_OF_SOURCE_DIR="$1";; + --quiet|-q) QUIET=1;; --random-seed) unset SEED;; --release-test|-r) SEED=1;; --seed|-s) shift; SEED="$1";; @@ -449,7 +459,7 @@ pre_setup_keep_going () { failure_summary="$failure_summary $text" failure_count=$((failure_count + 1)) - echo "${start_red}^^^^$text^^^^${end_color}" + echo "${start_red}^^^^$text^^^^${end_color}" >&2 fi } make () { @@ -495,6 +505,18 @@ not() { ! "$@" } +pre_setup_quiet_redirect () { + if [ $QUIET -ne 1 ]; then + redirect_out () { + "$@" + } + else + redirect_out () { + "$@" >/dev/null + } + fi +} + pre_prepare_outcome_file () { case "$MBEDTLS_TEST_OUTCOME_FILE" in [!/]*) MBEDTLS_TEST_OUTCOME_FILE="$PWD/$MBEDTLS_TEST_OUTCOME_FILE";; @@ -505,6 +527,10 @@ pre_prepare_outcome_file () { } pre_print_configuration () { + if [ $QUIET -eq 1 ]; then + return + fi + msg "info: $0 configuration" echo "MEMORY: $MEMORY" echo "FORCE: $FORCE" @@ -579,6 +605,11 @@ pre_check_tools () { "$ARMC6_CC" "$ARMC6_AR" "$ARMC6_FROMELF";; esac + # past this point, no call to check_tool, only printing output + if [ $QUIET -eq 1 ]; then + return + fi + msg "info: output_env.sh" case $RUN_COMPONENTS in *_armcc*) @@ -1889,10 +1920,15 @@ run_component () { dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 >/dev/null 2>&1 # Run the component code. - "$@" + if [ $QUIET -eq 1 ]; then + # msg() is silenced, so just print the component name here + echo "${current_component#component_}" + fi + redirect_out "$@" # Restore the build tree to a clean state. cleanup + current_component="" } # Preliminary setup @@ -1910,6 +1946,7 @@ else "$@" } fi +pre_setup_quiet_redirect pre_prepare_outcome_file pre_print_configuration pre_check_tools From dfb114a84307d2fe292db643115ddf7eda967563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jun 2020 11:40:08 +0200 Subject: [PATCH 3/7] Make check_generate_test_code more -q friendly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index fb9e50f99..e89e5ea27 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1886,7 +1886,10 @@ component_check_python_files () { component_check_generate_test_code () { msg "uint test: generate_test_code.py" - record_status ./tests/scripts/test_generate_test_code.py + # unittest writes out mundane stuff like number or tests run on stderr. + # Our convention is to reserve stderr for actual errors, and write + # harmless info on stdout so it can be suppress with --quiet. + record_status ./tests/scripts/test_generate_test_code.py 2>&1 } ################################################################ From a9119167e0739ca77b0ac24804618a8b8bfaaa53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jun 2020 11:51:40 +0200 Subject: [PATCH 4/7] Make component_check_test_cases more -q frienly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index e89e5ea27..1640dbe48 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -675,7 +675,12 @@ component_check_names () { component_check_test_cases () { msg "Check: test case descriptions" # < 1s - record_status tests/scripts/check-test-cases.py + if [ $QUIET -eq 1 ]; then + OPT='--quiet' + else + OPT='' + fi + record_status tests/scripts/check-test-cases.py $OPT } component_check_doxygen_warnings () { From 129e13cb1251e184c4848ec03d34c457f8bca3ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jun 2020 11:54:25 +0200 Subject: [PATCH 5/7] Use all.sh in pre-push hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list in the pre-push hook was redundant with the list of `check_*` components in all.sh, and unsurprisingly it was outdated. Missing components were: - check_recursion - check_changelog - check_test_cases - check_python_files - check_generate_test_code Signed-off-by: Manuel Pégourié-Gonnard --- tests/git-scripts/pre-push.sh | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tests/git-scripts/pre-push.sh b/tests/git-scripts/pre-push.sh index 86edf5a30..132e0b01a 100755 --- a/tests/git-scripts/pre-push.sh +++ b/tests/git-scripts/pre-push.sh @@ -32,18 +32,4 @@ echo "URL is $URL" set -eu -run_test() -{ - TEST=$1 - echo "running '$TEST'" - if ! `$TEST > /dev/null 2>&1`; then - echo "test '$TEST' failed" - return 1 - fi -} - -run_test ./tests/scripts/check-doxy-blocks.pl -run_test ./tests/scripts/check-names.sh -run_test ./tests/scripts/check-generated-files.sh -run_test ./tests/scripts/check-files.py -run_test ./tests/scripts/doxygen.sh +tests/scripts/all.sh -q -k 'check_*' From f1f180a6a19408023b04fdb2361c404c378006fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 8 Jun 2020 10:46:35 +0200 Subject: [PATCH 6/7] all.sh: keep dd output in non-quiet mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since dd prints everything on stderr, both normal status update and actual errors when they occur, redirecting that to /dev/null is a trade-off that's acceptable in quiet mode (typically used on a developer's machine and the developer will re-run in non-quiet mode if anything fails without sufficient detail in the output), but not that much in non-quiet mode. For example, if our dd invocation fails because the disk in full on a CI machine, we want the error to be reported at the time we invoke dd, and not later when a seemingly unrelated test fails due to an incorrect seedfile. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1640dbe48..f2b346359 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -510,10 +510,16 @@ pre_setup_quiet_redirect () { redirect_out () { "$@" } + redirect_err () { + "$@" + } else redirect_out () { "$@" >/dev/null } + redirect_err () { + "$@" 2>/dev/null + } fi } @@ -1925,7 +1931,7 @@ run_component () { # Unconditionally create a seedfile that's sufficiently long. # Do this before each component, because a previous component may # have messed it up or shortened it. - dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 >/dev/null 2>&1 + redirect_err dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 # Run the component code. if [ $QUIET -eq 1 ]; then From 304b0995342d7b77b3817f74f864633d09e9e7a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 8 Jun 2020 10:59:41 +0200 Subject: [PATCH 7/7] all.sh: clean up some uses of "local" variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While pure sh doesn't have a concept of local variables, we can partially emulate them by unsetting variables before we exit the function, and use the convention of giving them lowercase names to distinguish from global variables. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index f2b346359..32efc1d1f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -682,11 +682,12 @@ component_check_names () { component_check_test_cases () { msg "Check: test case descriptions" # < 1s if [ $QUIET -eq 1 ]; then - OPT='--quiet' + opt='--quiet' else - OPT='' + opt='' fi - record_status tests/scripts/check-test-cases.py $OPT + record_status tests/scripts/check-test-cases.py $opt + unset opt } component_check_doxygen_warnings () { @@ -1942,7 +1943,7 @@ run_component () { # Restore the build tree to a clean state. cleanup - current_component="" + unset current_component } # Preliminary setup