Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rostest results deleted by concurrent test run #1359

Open
jschleicher opened this issue Apr 9, 2018 · 4 comments
Open

Rostest results deleted by concurrent test run #1359

jschleicher opened this issue Apr 9, 2018 · 4 comments

Comments

@jschleicher
Copy link

If I run a test case concurrently from different console windows (e.g. to catch an unfrequent timing issue), I get rostest failures although the underlying gtest succeeded:

The error message is: "did not generate test results"

The problem seems to be: Every test instance starts its own roscore on separate ports, but the file name for the test results is always the same:

test_file = xmlResultsFile(test_pkg, test_name, False, env=env)

I'd suggest do add the roscore port number to the xml filename to circumvent this problem.

@dirk-thomas dirk-thomas added this to the untargeted milestone Apr 27, 2018
@dirk-thomas
Copy link
Member

It is currently not supported to run the tests concurrently. The naming of the result file is just one symptom. Tests might also in various other ways not be safe to be run concurrently. So it is unlikely that this use case will be supported in the future.

Please feel free to contribute a pull request to support this use case.

@cellard0or
Copy link

I just stumbled over this because my tests are run in parallel, too. Otherwise it would be too much of an impediment. A fix to this problem is fairly easy (just find a unique name for all rostests in a package), but I have some thoughts about this issue:

  • Is there any actual concern left which prohibits running rostests in parallel, from a ROS point of view? Since the roscore is started with separate ports it at least looks like it was the intention to allow this. Other shared resources might cause trouble but this would not be ROS' responsibility.
  • The test result file name collision is a problem even without race conditions. It means I won't be able to inspect the test results of all but the last one executed.
  • I am not sure how a proper fix could look like. Using the roscore port number for instance might also be unsafe since the port might get reused. I think that most consequently, there should be a check in xml_results_file which makes sure that it not already exists. Even better, some static checker could make sure all test names are unique in a package, maybe this would be a job for catkin_lint?

@mikepurvis
Copy link
Member

mikepurvis commented Sep 27, 2019

Any package with multiple Gazebo-based tests is going to have significant issues as the Gazebo port is currently fixed (see previous discussion on this under #903). This is just one example of the kind of external dependency that kills parallelism for ROS testing. Conceivably a lightweight container framework (or even LD_PRELOAD port redirection hackery) could address these kind of thing, but it would be a game of whack-a-mole to find and deal with the particular cases.

Certainly there are lots of scenarios where it's useful to be able to run a bunch of tests at once, particularly if you want to be able to do things like check for breakages in your dependencies pre-merge (which ros_buildfarm doesn't currently make any effort toward).

Anyway, if you pursue this, it's likely you'd be looking at a tooling level solution rather than something that could be built into catkin itself. For example, building the tests target to produce all the testing binaries, and then fanning out a containerized instance for the running of each individual run_tests_x target. But even this can be tricky because it's not always clear looking at the make help output from any particular package which targets are actually distinct tests vs grouping ones that just depend on other targets. I never really made meaningful progress on it, but this kind of thing was my intent with the catkin_tools_test plugin, though at this point it would probably make sense to start it over as an colcon plugin: https://github.com/mikepurvis/catkin_tools_test

@cellard0or
Copy link

@mikepurvis Thanks for the suggestion, I will take a look.

Coming back to the original issue, during investigation I found that the reason is not the concurrency, actually. The problem arises when two tests (the test-name parameter in the launch file) have the same name. Even if they are not in the same launch file and all tests are run sequentially, one test will overwrite the result of the other because the result file names collide. This also results in one less test being reported by catkin_test_results than actually exist.

I see two options to fix this:

  • Either make catkin (or another tool) automatically check that all test-name parameters in a package are unique. Maybe as part of catkin_lint?
  • Or check if a file with the same name already exists when generating the test result file and rename accordingly. Maybe just appending to the existing file instead would also be an option.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

4 participants