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

add CMake function ament_add_test_label() #240

Merged
merged 1 commit into from
Apr 16, 2020

Conversation

dirk-thomas
Copy link
Contributor

As syntactic sugar for ros2/ros2#900.

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas added the enhancement New feature or request label Apr 16, 2020
@dirk-thomas dirk-thomas self-assigned this Apr 16, 2020
Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

The code LGTM; though personally I would more likely call this once

set_tests_properties(testname PROPERTIES LABELS "xfail;etc")

than append multiple labels with

ament_add_test_label(testname xfail)
ament_add_test_label(testname etc)

since I'll forget what it means in the future, and it's fewer steps to google set_tests_properities() than it is to google ament_add_test_label() followed by set_tests_properites().

@dirk-thomas
Copy link
Contributor Author

though personally I would more likely call this once

But there are many cases were your CMake code in a package doesn't control the test target creation, e.g. https://github.com/ament/ament_index/blob/bfb1aa6f9f958561db616a9fcb84ca9e81e7fef3/ament_index_cpp/CMakeLists.txt#L44

That test target already has a label - gtest. In other cases like the linters the test has labels like linter;uncrustify.

If you set your own label on these tests using set_tests_properties() you will overwrite the existing labels.

@dirk-thomas dirk-thomas merged commit 4e247cd into master Apr 16, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/ament_add_test_label branch April 16, 2020 18:29
@dirk-thomas
Copy link
Contributor Author

than append multiple labels with

ament_add_test_label(testname xfail)
ament_add_test_label(testname etc)

You can also pass multiple labels to the function: ament_add_test_label(testname xfail etc).

j-rivero pushed a commit that referenced this pull request Apr 27, 2020
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.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
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants