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

ModuleNotFoundError python unit test #233

Open
imcelroy opened this issue Jan 7, 2025 · 13 comments
Open

ModuleNotFoundError python unit test #233

imcelroy opened this issue Jan 7, 2025 · 13 comments

Comments

@imcelroy
Copy link

imcelroy commented Jan 7, 2025

Hello,

I am having errors when running colcon test for unit tests of python modules that use parameters generated by generate_parameter_library.

I have created a simple unit test that reproduces this error using your example_python on this branch: bug/python-example-unit-test. It simply imports the minimal_publisher module.

Here are the steps to reproduce the error:

  1. rosdep update
  2. rosdep install --rosdistro humble
  3. colcon build --cmake-args -DBUILD_TESTING=1 -DCMAKE_EXPORT_COMPILE_COMMANDS=On --event-handlers console_direct+ --merge-install
  4. colcon test --merge-install --packages-select generate_parameter_module_example --event-handlers console_cohesion+ log_command+ summary+ --return-code-on-test-failure

The result should give something like the following:

==================================== ERRORS ====================================
________________________ ERROR collecting test session _________________________
/usr/lib/python3/dist-packages/pluggy/hooks.py:286: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
/usr/lib/python3/dist-packages/pluggy/manager.py:92: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
/usr/lib/python3/dist-packages/pluggy/manager.py:83: in <lambda>
    self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
/usr/lib/python3/dist-packages/_pytest/python.py:200: in pytest_collect_file
    module: Module = ihook.pytest_pycollect_makemodule(path=path, parent=parent)
/usr/lib/python3/dist-packages/pluggy/hooks.py:286: in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
/usr/lib/python3/dist-packages/pluggy/manager.py:92: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
/usr/lib/python3/dist-packages/pluggy/manager.py:83: in <lambda>
    self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
/opt/ros/humble/lib/python3.10/site-packages/launch_testing/pytest/hooks.py:193: in pytest_pycollect_makemodule
    entrypoint = find_launch_test_entrypoint(path)
/opt/ros/humble/lib/python3.10/site-packages/launch_testing/pytest/hooks.py:186: in find_launch_test_entrypoint
    module = path.pyimport()
/usr/lib/python3/dist-packages/py/_path/local.py:704: in pyimport
    __import__(modname)
/usr/lib/python3/dist-packages/_pytest/assertion/rewrite.py:170: in exec_module
    exec(co, module.__dict__)
test/test_parameters.py:3: in <module>
    import generate_parameter_module_example.minimal_publisher
generate_parameter_module_example/minimal_publisher.py:30: in <module>
    from generate_parameter_module_example.admittance_parameters import (
E   ModuleNotFoundError: No module named 'generate_parameter_module_example.admittance_parameters'
- generated xml file: /home/user/ws/build/generate_parameter_module_example/pytest.xml -
=========================== short test summary info ============================
ERROR  - ModuleNotFoundError: No module named 'generate_parameter_module_exam...
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 0.11s ===============================

Note: It seems to work if you run pytest directly inside the test/ folder.

@christophfroehlich I'm not sure who to tag, but I appreciate any help with this. Thanks!

(possible related issues: #192, #141)

Context:

  • Ros Distro: Humble
  • Ubuntu 22.04
@imcelroy
Copy link
Author

imcelroy commented Jan 8, 2025

After a bit more digging, it seems to me that colcon test adds the following paths to the beginning of the PYTHONPATH:

  • '/home/user/workspace/src/generate_parameter_library/example_python/test'
  • '/home/user/workspace/src/generate_parameter_library/example_python'

I think what is happening is that Python finds the module in the source directory when importing generate_parameter_module_example.minimal_publisher, and this directory doesn't contain the parameter module that is built in workspace/build and installed in workspace/install. Therefore, the import fails.

@Raivias
Copy link

Raivias commented Jan 9, 2025

+1 on this issue. I'll circle-back if I find a solution.

@sea-bass
Copy link
Collaborator

Probably a good idea hold the 0.4.0 release until this one is solved? Would be nice to have the fix in.

@imcelroy
Copy link
Author

Hello @Raivias and @sea-bass, thank you both for taking a look!

I pushed a commit (forssea-robotics@b1a3abd) that fixes the issue for me. I'm not convinced it is the best solution, but at least it is a work around. The fix installs the parameter module to the source directory, in addition to the build and install locations, if the user explicitly sets src_dir. If it isn't set, the current behavior is used in order to avoid breaking changes. Let me know what you think!

@MarqRazz
Copy link
Contributor

I can confirm that if I run a a test that includes generated parameters it fails, but if I build with --symlink-install (which copies the generated parameter file into the source directory) my test pass.

@sea-bass
Copy link
Collaborator

sea-bass commented Jan 10, 2025

@imcelroy I'm worried about that workaround mostly because installing to the source directory will be quite annoying for source control purposes.

Not knowing much about the internals... could another workaround be to specifically add to the PYTHONPATH in the unit tests themselves?

So in the Python tests, to do sys.path.append(<something relative to one of those ament package functions>)

But of course, there's probably a better way which requires doing some homework.

@sea-bass
Copy link
Collaborator

sea-bass commented Jan 11, 2025

OK I was able to reproduce the test now.

Also, turns out that I had approved #207, but I think this actually breaks the install location and we need to revert it. Could someone confirm? -- I made #236


What is described in #233 (comment) is exactly what is happening.

Weirdly enough, it seems like modifying sys.path in the tests has no effect? Even if I completely clear the path, I get this as the file that the module was imported from:

/home/sebastian/workspace/gpl_ws/src/generate_parameter_library/example_python/generate_parameter_module_example/__init__.py

note this is the src folder!

I think at this point the Python path doesn't even matter because the current folder is the src/generate_parameter_library/example_python folder so it has local visibility of the module regardless.

Indeed, if you move that unit test to any other package and run it, the test will actually succeed! Because colcon test is not adding the src folder to the path, AND the current folder is different so it's not at the same level as the source of the module (which of course does not have the ungenerated parameter module unless you --symlink-install).

This might be largely a Python issue we can't easily fix... testing from other packages works, at least?

@sea-bass
Copy link
Collaborator

sea-bass commented Jan 11, 2025

One thing that does work is actually to not have an __init__.py file in the source folder, but instead to generate it into install space!

This works great for me... check out this PR? #237

@imcelroy
Copy link
Author

Also, turns out that I had approved #207, but I think this actually breaks the install location and we need to revert it. Could someone confirm? -- I made #236

@sea-bass I'm not sure this breaks it. Before the PR it was installing it to a location without an __init__.py, therefore it definitely wasn't visible by Python. After the change, it installs to the same location as the other modules, which contains an __init__.py.

What is described in #233 (comment) is exactly what is happening.

Weirdly enough, it seems like modifying sys.path in the tests has no effect? Even if I completely clear the path, I get this as the file that the module was imported from:

/home/sebastian/workspace/gpl_ws/src/generate_parameter_library/example_python/generate_parameter_module_example/__init__.py

note this is the src folder!

I think at this point the Python path doesn't even matter because the current folder is the src/generate_parameter_library/example_python folder so it has local visibility of the module regardless.

Yes, I tried modifying the pythonpath as well and nothing seemed to work.

Indeed, if you move that unit test to any other package and run it, the test will actually succeed! Because colcon test is not adding the src folder to the path, AND the current folder is different so it's not at the same level as the source of the module (which of course does not have the ungenerated parameter module unless you --symlink-install).

This might be largely a Python issue we can't easily fix... testing from other packages works, at least?

I agree that it seems more like a Python/Colcon issue, however it is a big limitation and implies that we can't write tests for a package inside the package if it uses generate_parameter_library...

One thing that does work is actually to not have an init.py file in the source folder, but instead to generate it into install space!

I'll try this out, but I'm worried about this solution too because according to the ROS examples (and pure python examples) the __init__.py should be placed in the source folder where your modules are created.

@sea-bass sea-bass reopened this Jan 14, 2025
@sea-bass
Copy link
Collaborator

I'll reopen this.

@imcelroy If you still do have an __init__.py in the source folder, this shouldn't break anything, right? Except for running the unit tests?

If that's the case, I'd favor carrying on with that 0.4.0 release and continuing to iterate with more ideas. I'm happy to help test.

@imcelroy
Copy link
Author

imcelroy commented Jan 14, 2025

I'll reopen this.

@imcelroy If you still do have an __init__.py in the source folder, this shouldn't break anything, right? Except for running the unit tests?

If that's the case, I'd favor carrying on with that 0.4.0 release and continuing to iterate with more ideas. I'm happy to help test.

@sea-bass That's correct, if we keep the __init__.py in the source folder as is suggested by ROS and Python, everything is fine except for the unit tests. However, that means this will break existing unit tests for users that are trying to adopt generate_parameter_library into an existing ROS2 python package (like myself) after having followed ROS guidelines.

Maybe we should be generating and installing the parameters as a python module separate from the current package. For example:

from generate_parameter_module_example.admittance_parameters import (
    admittance_controller,
)

Would become something like:

from generate_parameter_module_example_admittance_parameters import (
    admittance_controller,
)

Or:

from generate_parameter_module_example_parameters.admittance_parameters import (
    admittance_controller,
)

This way, the parameters would be found in every case, and there would be no source control issues.

@Raivias
Copy link

Raivias commented Jan 17, 2025

After reading @MarqRazz 's comment I can confirm my issue was that I wasn't using the --symlink-install flag when building. I also didn't find it in the documentation.

@Benjamin-Tan
Copy link

From https://github.com/colcon/colcon-core/blob/master/colcon_core/task/python/build.py, the args.install_base doesn't seem to be passed down as command arg for the packages to be built. I understand the idea of aligning the C++ and Python parameter generation where the generated code is stored to the build folder directly.

But would it be easier and more straightforward for the generated code for Python to be in the source folder directly? This would simplify and perhaps solve the issue of symlink, different install_path, merge install and etc.

The current location to store the generated code is also not ideal as the build/install path may be different depending on their colcon arguments/configurations as build-base, install-base

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

No branches or pull requests

5 participants