Run 'colcon test-result' to print test failures

[christophebedard]

Closes #784

action-ros-ci currently runs colcon test ... with --return-code-on-test-failure, which makes the CI job fail right after that command if any tests fail. Users need to look through the test output to find the test failures, which is a bit annoying.

Instead, remove the --return-code-on-test-failure option from the colcon test ... command, and use colcon test-result instead. Run it first with the --all option so that all test results are printed, including the tests that passed. This allows quickly checking what tests ran. Since colcon test-result returns a non-zero error code if any tests failed, ignore the return code here. Then, run colcon test-result with the --verbose option (only) to show only test results of failed tests along with part of their output. This time, the return code isn't ignored, which makes the CI job fail if there are any failures. This is much closer to the way CI jobs work on ci.ros2.org.

This shouldn't affect the actual outcome of CI jobs; it should only affect the output.

Signed-off-by: Christophe Bedard christophe.bedard@apex.ai

[codecov]

Codecov Report

Base: 48.73% // Head: 47.87% // Decreases project coverage by -0.87% ⚠️

Coverage data is based on head (0bd8c9a) compared to base (e9721b4).
Patch coverage: 0.00% of modified lines in pull request are covered.

❗ Current head 0bd8c9a differs from pull request most recent head 04119b0. Consider uploading reports for the commit 04119b0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #794      +/-   ##
==========================================
- Coverage   48.73%   47.87%   -0.87%     
==========================================
  Files           2        2              
  Lines         277      282       +5     
  Branches       76       76              
==========================================
  Hits          135      135              
- Misses        142      147       +5     

Impacted Files	Coverage Δ
src/action-ros-ci.ts	41.89% <0.00%> (-0.85%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[emersonknapp]

Great - that's a nice usability improvement, I'm always searching through the long long logs for what failed.

[christophebedard]

Great - that's a nice usability improvement, I'm always searching through the long long logs for what failed.

ci.ros2.org doesn't use --verbose with colcon test-result, but you think I should use it here? That would make it print the output (or some excerpt of it?) of failed tests, so you'd see right away what caused the failure. Example, adding a random space to make the linters fail:

Example

$ colcon test --packages-select tracetools
$ colcon test-result --all
build/tracetools/Testing/20230215-1714/Test.xml: 8 tests, 0 errors, 2 failures, 0 skipped
build/tracetools/test_results/tracetools/copyright.xunit.xml: 11 tests, 0 errors, 0 failures, 0 skipped
build/tracetools/test_results/tracetools/cppcheck.xunit.xml: 11 tests, 0 errors, 0 failures, 11 skipped
build/tracetools/test_results/tracetools/cpplint.xunit.xml: 11 tests, 0 errors, 1 failure, 0 skipped
build/tracetools/test_results/tracetools/lint_cmake.xunit.xml: 1 test, 0 errors, 0 failures, 0 skipped
build/tracetools/test_results/tracetools/test_status.gtest.xml: 1 test, 0 errors, 0 failures, 0 skipped
build/tracetools/test_results/tracetools/uncrustify.xunit.xml: 11 tests, 0 errors, 1 failure, 0 skipped
build/tracetools/test_results/tracetools/xmllint.xunit.xml: 1 test, 0 errors, 0 failures, 0 skipped

Summary: 55 tests, 0 errors, 4 failures, 11 skipped
$ colcon test-result --verbose 
build/tracetools/Testing/20230215-1714/Test.xml: 8 tests, 0 errors, 2 failures, 0 skipped
- cpplint
  <<< failure message
    -- run_test.py: invoking following command in '/home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools':
     - /home/chris/ros2_ws/install/ament_cpplint/bin/ament_cpplint --xunit-file /home/chris/ros2_ws/build/tracetools/test_results/tracetools/cpplint.xunit.xml
    /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/utils.cpp:23:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
    Category 'whitespace/end_of_line' errors found: 1
    Total errors found: 1
    Using '--root=/home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/include' argument
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/include/tracetools/status.h
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/include/tracetools/tp_call.h
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/include/tracetools/tracetools.h
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/include/tracetools/utils.hpp
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/include/tracetools/visibility_control.hpp
    
    Using '--root=/home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src' argument
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/status.c
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/status_tool.c
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/tp_call.c
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/tracetools.c
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/utils.cpp
    
    Using '--root=/home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/test' argument
    
    Done processing /home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/test/test_status.cpp
    
    -- run_test.py: return code 1
    -- run_test.py: verify result file '/home/chris/ros2_ws/build/tracetools/test_results/tracetools/cpplint.xunit.xml'
  >>>
- uncrustify
  <<< failure message
    -- run_test.py: invoking following command in '/home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools':
     - /home/chris/ros2_ws/install/ament_uncrustify/bin/ament_uncrustify --xunit-file /home/chris/ros2_ws/build/tracetools/test_results/tracetools/uncrustify.xunit.xml
    Code style divergence in file 'src/utils.cpp':
    
    --- src/utils.cpp
    +++ src/utils.cpp.uncrustify
    @@ -23 +23 @@
    - 
    +
    
    1 files with code style divergence
    No code style divergence in file 'include/tracetools/status.h'
    
    No code style divergence in file 'include/tracetools/tp_call.h'
    
    No code style divergence in file 'include/tracetools/tracetools.h'
    
    No code style divergence in file 'include/tracetools/utils.hpp'
    
    No code style divergence in file 'include/tracetools/visibility_control.hpp'
    
    No code style divergence in file 'src/status.c'
    
    No code style divergence in file 'src/status_tool.c'
    
    No code style divergence in file 'src/tp_call.c'
    
    No code style divergence in file 'src/tracetools.c'
    
    No code style divergence in file 'test/test_status.cpp'
    
    -- run_test.py: return code 1
    -- run_test.py: verify result file '/home/chris/ros2_ws/build/tracetools/test_results/tracetools/uncrustify.xunit.xml'
  >>>
build/tracetools/test_results/tracetools/cpplint.xunit.xml: 11 tests, 0 errors, 1 failure, 0 skipped
- tracetools.cpplint whitespace/end_of_line [4] (/home/chris/ros2_ws/src/ros2/ros2_tracing/tracetools/src/utils.cpp:23)
  <<< failure message
    Line ends in whitespace.  Consider deleting these extra spaces.
  >>>
build/tracetools/test_results/tracetools/uncrustify.xunit.xml: 11 tests, 0 errors, 1 failure, 0 skipped
- tracetools.uncrustify src/utils.cpp
  <<< failure message
    Diff with 5 lines
  >>>

Summary: 55 tests, 0 errors, 4 failures, 11 skipped

Otherwise, with colcon test-result without --verbose, you only get the list of failed tests:

Example

$ colcon test-result 
build/tracetools/Testing/20230215-1714/Test.xml: 8 tests, 0 errors, 2 failures, 0 skipped
build/tracetools/test_results/tracetools/cpplint.xunit.xml: 11 tests, 0 errors, 1 failure, 0 skipped
build/tracetools/test_results/tracetools/uncrustify.xunit.xml: 11 tests, 0 errors, 1 failure, 0 skipped

Summary: 55 tests, 0 errors, 4 failures, 11 skipped

Then you do still have to go look at the "raw" test output.

cc @ijnek, since you might have an opinion on this too.

[emersonknapp]

Yeah, I think it's good like that! As long as it's just printing failed tests and not all tests outputs

[christophebedard]

Yeah, I think it's good like that! As long as it's just printing failed tests and not all tests outputs

Yep, first colcon test-result --all prints a list of all tests that ran, then colcon test-result --verbose prints a list of tests that failed along with part of their output.

[ijnek]

I'm late, but the changes are good. Thanks @christophebedard !