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

[noetic] wrap rostest call to add python pointing to sys.executable in PATH #1879

Merged
merged 5 commits into from
Feb 21, 2020

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Feb 7, 2020

Another possible solution to #1830. This adds a temporary directory with a symlink to python to the path variable.

This uses tempfile.TemporaryDirectory which is only available in python 3.2 and above, so it must not be merged onto the melodic-devel branch. With this PR (and #1870) I don't see any ros_comm tests using Python 2, though many tests raise ModuleNotFoundError when trying to import other packages.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz self-assigned this Feb 7, 2020
@dirk-thomas dirk-thomas changed the title Wrap rostest call to set python in PATH [noetic] wrap rostest call to add python pointing to sys.executable in PATH Feb 7, 2020
@dirk-thomas
Copy link
Member

though many tests raise ModuleNotFoundError when trying to import other packages.

Can you please reference some of the failing tests here to be able to determine what might cause this.

@sloretz
Copy link
Contributor Author

sloretz commented Feb 7, 2020

Can you please reference some of the failing tests here to be able to determine what might cause this.

======================================================================
ERROR: test_validate_master_launch (unit.test_roslaunch_launch.TestRoslaunchLaunch)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sloretz/ws/noetic/src/ros_comm/tools/roslaunch/test/unit/test_roslaunch_launch.py", line 47, in test_validate_master_launch
    import roslaunch.launch
  File "/home/sloretz/ws/noetic/src/ros_comm/tools/roslaunch/src/roslaunch/__init__.py", line 62, in <module>
    from .scriptapi import ROSLaunch
  File "/home/sloretz/ws/noetic/src/ros_comm/tools/roslaunch/src/roslaunch/scriptapi.py", line 42, in <module>
    import roslaunch.parent
  File "/home/sloretz/ws/noetic/src/ros_comm/tools/roslaunch/src/roslaunch/parent.py", line 54, in <module>
    import roslaunch.server
  File "/home/sloretz/ws/noetic/src/ros_comm/tools/roslaunch/src/roslaunch/server.py", line 79, in <module>
    from rosgraph_msgs.msg import Log
ModuleNotFoundError: No module named 'rosgraph_msgs'

@dirk-thomas
Copy link
Member

dirk-thomas commented Feb 7, 2020

That test (roslaunch/test/unit/test_roslaunch_launch.py) isn't run by rostest but nose, right? So this change wouldn't have any effect.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Copy link
Contributor Author

sloretz commented Feb 7, 2020

That test (roslaunch/test/unit/test_roslaunch_launch.py) isn't run by rostest but nose, right? So this change wouldn't have any effect.

Yeah it seems unrelated to this PR. rosgraph_msgs doesn't appear to be part of the underlay when building with catkin_make_isolated. No clue why yet, but I'll move that to a separate issue.

@dirk-thomas
Copy link
Member

rosgraph_msgs doesn't appear to be part of the underlay when building with catkin_make_isolated.

I am surprised since it is a declared dependency:

<build_depend>rosgraph_msgs</build_depend>

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

lgtm

@sloretz
Copy link
Contributor Author

sloretz commented Feb 18, 2020

@sloretz
Copy link
Contributor Author

sloretz commented Feb 19, 2020

@ros-pull-request-builder retest this please

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

Just waiting for the noetic-devel branch to be created.

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

Successfully merging this pull request may close these issues.

None yet

2 participants