Skipped tests #80
Skipped tests #80
Conversation
|
CI job testing this and the following PRs ros2/rcl#90, ament/ament_tools#125, ros2/system_tests#177 |
| REQUIRED_FILES "${executable}" | ||
| LABELS "gtest" | ||
| ) | ||
| endif() |
There was a problem hiding this comment.
Instead of duplicating the function call with all its arguments I would suggest to use a variable to either pass or not pass SKIP_TEST.
There was a problem hiding this comment.
good point: 17ae941
There was a problem hiding this comment.
The same applies to the other function in this patch: https://github.com/ament/ament_cmake/pull/80/files#diff-29a991acf62a8fc4460ed46cdb27c46eR113
There was a problem hiding this comment.
thanks 91b19d2
| skipped_result_file = _generate_result( | ||
| args.result_file, | ||
| None, | ||
| True |
There was a problem hiding this comment.
Keyword arguments should be passed with the keyword rather then as a positional argument. Then you also don't need the default value for failure_message.
There was a problem hiding this comment.
thanks, using keyword arguments everywhere now 9c22bc9
| @@ -231,20 +246,30 @@ def log(msg, **kwargs): | |||
| return rc | |||
|
|
|||
|
|
|||
| def _generate_result(result_file, failure_message=None): | |||
| def _generate_result(result_file, failure_message=None, skip=False): | |||
There was a problem hiding this comment.
To avoid similar problems in the future the signature could be changed to:
def _generate_result(result_file, *, failure_message=None, skip=False):
There was a problem hiding this comment.
hooray Python3 argument parsing. 3d92b6b
|
CI for this and all the associated PRs: I don't think the BlackBox failing tests on OSX are related to this PR |
|
When adding new arguments please also update the corresponding docblock. See 9f84a82 |
|
Sorry about that, I'll push an update shortly |
|
I already added the doc lines in the referenced commit. |
|
Yeah I meant the other files that got SKIP_TEST argument added. See bf69fdd |
|
I just added them to the existing PR: 955f2df |
|
great thanks, deleting the other branch then |
|
Thanks, sorry for the overlap 😉 |
generate skipped result files + add skip option to all add_test macros