Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark skipped SLI tests as skipped, not success #713

Merged
merged 7 commits into from
May 27, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/sli/unittest.sli
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ is_threaded_with_openmp not { exit_test_gracefully } if
*/
/exit_test_gracefully
{
statusdict/exitcodes/success :: quit_i
statusdict/exitcodes/skipped :: quit_i
} def


Expand Down
2 changes: 2 additions & 0 deletions sli/slistartup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ SLIStartup::SLIStartup( int argc, char** argv )
, ndebug_name( "ndebug" )
, exitcodes_name( "exitcodes" )
, exitcode_success_name( "success" )
, exitcode_skipped_name( "skipped" )
, exitcode_scripterror_name( "scripterror" )
, exitcode_abort_name( "abort" )
, exitcode_userabort_name( "userabort" )
Expand Down Expand Up @@ -538,6 +539,7 @@ SLIStartup::init( SLIInterpreter* i )

exitcodes->insert(
exitcode_success_name, Token( new IntegerDatum( EXIT_SUCCESS ) ) );
exitcodes->insert( exitcode_skipped_name, Token( new IntegerDatum( 200 ) ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it is not good practice to have raw numbers in the code. Named constants should be used instead.

exitcodes->insert(
exitcode_scripterror_name, Token( new IntegerDatum( 126 ) ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be systematic, turn this and the other numeric values into preprocessor constants, too.

exitcodes->insert(
Expand Down
1 change: 1 addition & 0 deletions sli/slistartup.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class SLIStartup : public SLIModule

Name exitcodes_name;
Name exitcode_success_name;
Name exitcode_skipped_name;
Name exitcode_scripterror_name;
Name exitcode_abort_name;
Name exitcode_userabort_name;
Expand Down
42 changes: 26 additions & 16 deletions testsuite/do_tests.sh.in
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ run_test ()

param_script="$1"
param_success="$2"
param_failure="$3"
param_skipped="$3"
param_failure="$4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the documentation of run_test() above to include param_skipped.


msg_error=

Expand Down Expand Up @@ -288,11 +289,18 @@ run_test ()
sed 's/^/ > /g' "${TEST_OUTFILE}" >> "${TEST_LOGFILE}"

msg_dirty=${param_success##* ${exit_code} }
msg_dirty_skip=${param_skipped##* ${exit_code} }
msg_clean=${msg_dirty%%,*}
if test "${msg_dirty}" != "${param_success}" ; then
TEST_PASSED=$(( ${TEST_PASSED} + 1 ))
explanation="${msg_clean}"
junit_failure=
elif test "${msg_dirty_skip}" != "${param_skipped}" ; then
TEST_PASSED=$(( ${TEST_PASSED} + 1 ))
msg_dirty=${param_skipped##* ${exit_code} }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right hand side here is the same as on line 292, maybe no need to repeat?

msg_clean=${msg_dirty%%,*}
explanation="${msg_clean}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this code right, then this section handles skipped tests. The Travis log shows that correct messages are shown for individual skipped tests, but the skipped tests should be counted as skipped, not as passed, and the final summary should show them as skipped.

junit_failure=
else
TEST_FAILED=$(( ${TEST_FAILED} + 1 ))
JUNIT_FAILURES=$(( ${JUNIT_FAILURES} + 1 ))
Expand Down Expand Up @@ -412,6 +420,8 @@ echo > "${TEST_LOGFILE}" "NEST v. @NEST_VERSION_VERSION@ testsuite log"
echo >> "${TEST_LOGFILE}" "======================"
echo >> "${TEST_LOGFILE}" "Running tests from ${TEST_BASEDIR}"

CODES_SKIPPED=' 200 Skipped'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about that but one may ask why these tests are skipped. CODES_SKIPPED=' 200 Skipped (build with-mpi=OFF required)' or something like that would make it clear. We have similar comments at other places, e.g. Success (expected failure).


echo
echo 'Phase 1: Testing if SLI can execute scripts and report errors'
echo '-------------------------------------------------------------'
Expand All @@ -421,13 +431,13 @@ junit_open 'core.phase_1'
CODES_SUCCESS=' 0 Success'
CODES_FAILURE=
for test_name in test_pass.sli test_goodhandler.sli test_lazyhandler.sli ; do
run_test "selftests/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "selftests/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done

CODES_SUCCESS=' 126 Success'
CODES_FAILURE=
for test_name in test_fail.sli test_stop.sli test_badhandler.sli ; do
run_test "selftests/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "selftests/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done

junit_close
Expand All @@ -449,48 +459,48 @@ junit_open 'core.phase_2'
CODES_SUCCESS=' 2 Success'
CODES_FAILURE=' 126 Failed: error in test script'

run_test selftests/test_pass_or_die.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_pass_or_die.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"

CODES_SUCCESS=' 1 Success'
CODES_FAILURE=\
' 2 Failed: error in tested code block,'\
' 126 Failed: error in test script,'

run_test selftests/test_assert_or_die_b.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_assert_or_die_p.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_assert_or_die_b.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
run_test selftests/test_assert_or_die_p.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"

CODES_SUCCESS=' 3 Success'
CODES_FAILURE=\
' 1 Failed: missed assertion,'\
' 2 Failed: error in tested code block,'\
' 126 Failed: error in test script,'

run_test selftests/test_fail_or_die.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_fail_or_die.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"

CODES_SUCCESS=' 3 Success'
CODES_FAILURE=\
' 1 Failed: missed assertion,'\
' 2 Failed: error in tested code block,'\
' 126 Failed: error in test script,'

run_test selftests/test_crash_or_die.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_crash_or_die.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"

CODES_SUCCESS=' 3 Success'
CODES_FAILURE=\
' 1 Failed: missed assertion,'\
' 2 Failed: error in tested code block,'\
' 126 Failed: error in test script,'

run_test selftests/test_failbutnocrash_or_die_crash.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_failbutnocrash_or_die_pass.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_failbutnocrash_or_die_crash.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
run_test selftests/test_failbutnocrash_or_die_pass.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"

CODES_SUCCESS=' 3 Success'
CODES_FAILURE=\
' 1 Failed: missed assertion,'\
' 2 Failed: error in tested code block,'\
' 126 Failed: error in test script,'

run_test selftests/test_passorfailbutnocrash_or_die.sli "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test selftests/test_passorfailbutnocrash_or_die.sli "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"

junit_close

Expand Down Expand Up @@ -524,7 +534,7 @@ junit_open 'core.phase_3'

for test_ext in sli py ; do
for test_name in $(ls "${TEST_BASEDIR}/unittests/" | grep ".*\.${test_ext}\$") ; do
run_test "unittests/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "unittests/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done
done

Expand All @@ -538,7 +548,7 @@ junit_open 'core.phase_4'

for test_ext in sli py ; do
for test_name in $(ls "${TEST_BASEDIR}/regressiontests/" | grep ".*\.${test_ext}$") ; do
run_test "regressiontests/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "regressiontests/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done
done

Expand All @@ -552,7 +562,7 @@ if test "x$(sli -c 'statusdict/have_mpi :: =')" = xtrue ; then

NEST_BINARY=nest_indirect
for test_name in $(ls "${TEST_BASEDIR}/mpi_selftests/pass" | grep '.*\.sli$') ; do
run_test "mpi_selftests/pass/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "mpi_selftests/pass/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done

# tests meant to fail
Expand All @@ -561,13 +571,13 @@ if test "x$(sli -c 'statusdict/have_mpi :: =')" = xtrue ; then
CODES_SUCCESS=' 1 Success (expected failure)'
CODES_FAILURE=' 0 Failed: Unittest failed to detect error.'
for test_name in $(ls "${TEST_BASEDIR}/mpi_selftests/fail" | grep '.*\.sli$') ; do
run_test "mpi_selftests/fail/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "mpi_selftests/fail/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done
CODES_SUCCESS=${SAVE_CODES_SUCCESS}
CODES_FAILURE=${SAVE_CODES_FAILURE}

for test_name in $(ls "${TEST_BASEDIR}/mpitests/" | grep '.*\.sli$') ; do
run_test "mpitests/${test_name}" "${CODES_SUCCESS}" "${CODES_FAILURE}"
run_test "mpitests/${test_name}" "${CODES_SUCCESS}" "${CODES_SKIPPED}" "${CODES_FAILURE}"
done

junit_close
Expand Down
2 changes: 1 addition & 1 deletion testsuite/regressiontests/ticket-235.sli
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
% preparatory work for proper test code in case NEST is complied with MPI support
% For now we just ignore this test, this will later be replaced
% by a restart of NEST with a serial binary.
statusdict/is_mpi :: {statusdict/exitcodes/success :: quit_i} if
statusdict/is_mpi :: {exit_test_gracefully} if


{load} failbutnocrash_or_die
Expand Down
8 changes: 4 additions & 4 deletions testsuite/regressiontests/ticket-464.sli
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ This test will only be executed if NEST has been compiled without MPI support.
Author: Hans Ekkehard Plesser, 2010-10-04
*/

(unittest) run
/unittest using

% preparatory work for proper test code in case NEST is complied with MPI support
% For now we just ignore this test, this will later be replaced
% by a restart of NEST with a serial binary.
statusdict/is_mpi :: {statusdict/exitcodes/success :: quit_i} if

(unittest) run
/unittest using
statusdict/is_mpi :: {exit_test_gracefully} if

M_ERROR setverbosity

Expand Down
2 changes: 1 addition & 1 deletion testsuite/regressiontests/ticket-681.sli
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ is_threaded not { exit_test_gracefully } if
% preparatory work for proper test code in case NEST is complied with MPI support
% For now we just ignore this test, this will later be replaced
% by a restart of NEST with a serial binary.
statusdict/is_mpi :: {statusdict/exitcodes/success :: quit_i} if
statusdict/is_mpi :: {exit_test_gracefully} if

M_ERROR setverbosity

Expand Down
2 changes: 1 addition & 1 deletion testsuite/regressiontests/ticket-787.sli
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ M_ERROR setverbosity
% preparatory work for proper test code in case NEST is complied with MPI support
% For now we just ignore this test, this will later be replaced
% by a restart of NEST with a serial binary.
statusdict/is_mpi :: {statusdict/exitcodes/success :: quit_i} if
statusdict/is_mpi :: {exit_test_gracefully} if


% entries to skip
Expand Down
9 changes: 4 additions & 5 deletions testsuite/regressiontests/ticket-926.sli
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,13 @@ if volume transmitter is not set.
Author: Hans Ekkehard Plesser, 2014-11-12
*/

(unittest) run
/unittest using

% preparatory work for proper test code in case NEST is complied with MPI support
% For now we just ignore this test, this will later be replaced
% by a restart of NEST with a serial binary.
statusdict/is_mpi :: {statusdict/exitcodes/success :: quit_i} if


(unittest) run
/unittest using
statusdict/is_mpi :: {exit_test_gracefully} if

M_ERROR setverbosity

Expand Down
2 changes: 1 addition & 1 deletion testsuite/unittests/test_fast_operators.sli
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Author: Diesmann
% preparatory work for proper test code in case NEST is complied with MPI support
% For now we just ignore this test, this will later be replaced
% by a restart of NEST with a serial binary.
statusdict/is_mpi :: {statusdict/exitcodes/success :: quit_i} if
statusdict/is_mpi :: {exit_test_gracefully} if


{add} failbutnocrash_or_die % good if nest survives
Expand Down