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
fix location of rosunit result files generated by rostests #668
fix location of rosunit result files generated by rostests #668
Conversation
Test passed. |
@ros/ros_team Please take a look to review this patch. |
lgtm |
test_file = xmlResultsFile(test_pkg, test_name, False) | ||
env = None | ||
if results_base_dir: | ||
env = {ROS_TEST_RESULTS_DIR: results_base_dir} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there potentially other env values that this is going to override or suppress? It might be better to append to the env instead of overriding it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other code uses the same pattern:
ros_comm/tools/rostest/src/rostest/rostest_main.py
Lines 148 to 154 in 44a089a
env = None | |
if options.results_base_dir: | |
env = {ROS_TEST_RESULTS_DIR: options.results_base_dir} | |
# #1140 | |
if not os.path.isfile(test_file): | |
results_file = xmlResultsFile(pkg, outname, True, env=env) |
And at the end of the call stack only this value is checked: https://github.com/ros-infrastructure/rospkg/blob/d3c12bdfbf990b9910220642bafef16b4c830b63/src/rospkg/environment.py#L162-L166
+1 |
1 similar comment
+1 |
fix location of rosunit result files generated by rostests
…osunit fix location of rosunit result files generated by rostests
…ostests) has been released ad 1.11.14 see https://github.com/ros/ros_comm/blob/melodic-devel/tools/rostest/CHANGELOG.rst#11114-2015-09-19
…ostests) has been released ad 1.11.14 see https://github.com/ros/ros_comm/blob/melodic-devel/tools/rostest/CHANGELOG.rst#11114-2015-09-19
Based on the discussion from #666 (comment)