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

gmock - Don't pass FALSE value to ament_add_test when SKIP_TEST is not provided #236

Merged

Conversation

emersonknapp
Copy link
Contributor

@emersonknapp emersonknapp commented Apr 3, 2020

CMake parsed "option" arguments come out as TRUE or FALSE values.
When ament_add_gmock does not receive the SKIP_TEST option, it passes an unmarked "FALSE" to ament_add_test.

This reveals itself as a problem in any example that uses ENV (or any other multi-value argument)

ament_add_gmock(my_test
  some_test_file.cpp
  ENV RMW_IMPLEMENTATION=rmw_fastrtps_cpp)

This makes the ENV argument be parsed in ament_add_test as

RMW_IMPLEMENTATION=rmw_fastrtps_cpp;FALSE

This results in the last environment variable having ";FALSE" appended to it, e.g.

1: Test command: /usr/bin/python3 "-u" "/ws/install/ament_cmake_test/share/ament_cmake_test/cmake/run_test.py" "/ws/build/rosbag2_transport/test_results/rosbag2_transport/test_info__rmw_connext_cpp.gtest.xml" "--package-name" "rosbag2_tra
nsport" "--output-file" "/ws/build/rosbag2_transport/ament_cmake_gmock/test_info__rmw_connext_cpp.txt" "--env" "RMW_IMPLEMENTATION=rmw_connext_cpp" "FALSE" "--command" "/ws/build/rosbag2_transport/test_info__rmw_connext_cpp" "--gtest_outp
ut=xml:/ws/build/rosbag2_transport/test_results/rosbag2_transport/test_info__rmw_connext_cpp.gtest.xml"                                                                                                                                       
1: Test timeout computed to be: 60                                                                                                                                                                                                            
1: -- run_test.py: extra environment variables:                                                                                                                                                                                               
1:  - RMW_IMPLEMENTATION=rmw_connext_cpp;FALSE                                                                                                                                                                                                
1: -- run_test.py: invoking following command in '/ws/build/rosbag2_transport':                                                                                                                                                               
1:  - /ws/build/rosbag2_transport/test_info__rmw_connext_cpp --gtest_output=xml:/ws/build/rosbag2_transport/test_results/rosbag2_transport/test_info__rmw_connext_cpp.gtest.xml                                                               
1: [ERROR] [1585880528.876809338] [rcl]: Expected RMW implementation identifier of 'rmw_connext_cpp;FALSE' but instead found 'rmw_connext_cpp', exiting with 102.                  
  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: Emerson Knapp emerson.b.knapp@gmail.com

Copy link

@thomas-moulard thomas-moulard left a comment

Choose a reason for hiding this comment

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

LGTM!

@dirk-thomas
Copy link
Contributor

I moved the added line to the previous location to minimize the diff: df9814b.

@dirk-thomas
Copy link
Contributor

Can you please squash the commits to make the DCO happy (sorry, didn't think about that when doing the edit through the web interface).

@dirk-thomas dirk-thomas added the bug Something isn't working label Apr 3, 2020
@emersonknapp emersonknapp force-pushed the emersonknapp/fix-gmock-skip-tests branch from df9814b to 8b39227 Compare April 3, 2020 18:26
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp force-pushed the emersonknapp/fix-gmock-skip-tests branch from 8b39227 to a9cec39 Compare April 3, 2020 18:26
@emersonknapp
Copy link
Contributor Author

@dirk-thomas done, thanks for the review

@dirk-thomas
Copy link
Contributor

Thanks for the fix.

@dirk-thomas dirk-thomas merged commit c754316 into ament:master Apr 3, 2020
j-rivero pushed a commit that referenced this pull request Apr 27, 2020
…ed (#236)

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants