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

[roslaunch] roslaunch-check only considers "default" path #1394

Open
miguelprada opened this issue May 9, 2018 · 3 comments
Open

[roslaunch] roslaunch-check only considers "default" path #1394

miguelprada opened this issue May 9, 2018 · 3 comments

Comments

@miguelprada
Copy link

When conditionally including launch files based on input arguments, only the includes that are pulled in for the default values of the input arguments are considered by roslaunch-check.

The issue can be reproduced with the following launchfile which should, in my opinion, fail:

<launch>
  <arg name="fail" default="false"/>
  <include if="$(arg fail)" file="does/not/exist.launch"/>
</launch>

I know this directly contradicts #993/#998. I agree that in the simple example given in #993, it is not reasonable for roslaunch-check to fail, but in an example such as the one above, I would expect all possible paths to be tested.

I am aware that the possible paths is not something which can be dynamically computed, particularly since one can have arbitrary $(eval) substitution args which depend on arbitrary input arguments. Therefore I can't think of any way to properly cover the possible cases other than evaluating everything irrespectively of the conditional.

One alternative could be that we add test-only launch files to our packages, which do exercise all possible variants of the input arguments. However, the number of required test launch files [1] grows exponentially with the number of input arguments of the original launch file, and quickly becomes unmanageable.

[1] Instead of using multiple test launch files, one could think of using a single test launch file, which would include multiple copies of the launch file to be tested with different sets of input arguments but I found a separate issue about this which I'm about to post.

@mikepurvis
Copy link
Member

One alternative could be that we add test-only launch files to our packages, which do exercise all possible variants of the input arguments. However, the number of required test launch files [1] grows exponentially with the number of input arguments of the original launch file, and quickly becomes unmanageable.

IMO this is a reasonable approach to testing multiple possible configurations of a given launch file.

@miguelprada
Copy link
Author

One alternative could be that we add test-only launch files to our packages, which do exercise all possible variants of the input arguments. However, the number of required test launch files [1] grows exponentially with the number of input arguments of the original launch file, and quickly becomes unmanageable.

IMO this is a reasonable approach to testing multiple possible configurations of a given launch file.

You may well be right, but I was certainly surprised when I found an example like the one above not failing the tests (it took a different package including that launch file with fail set to true to trigger a test failure).

What would you think of adding some --ignore-conditionals-like input flag to roslaunch-check (and exposing it somehow in roslaunch_add_file_check)?

@130s
Copy link
Member

130s commented Jul 26, 2019

What would you think of adding some --ignore-conditionals-like input flag to roslaunch-check (and exposing it somehow in roslaunch_add_file_check)?

That might be reasonably practical given the complication discussed above.

@collin-scribner just hit similar/the same issue while tackling #953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants