ros2_documentation icon indicating copy to clipboard operation
ros2_documentation copied to clipboard

Regression - Python tests no longer discovered if using new setuptools.

Open Ryanf55 opened this issue 1 year ago • 2 comments

Description

My tests for an ament_python package are no longer being recognized. I went through a basic package creation, then followed the python testing tutorial.

When you run colcon test, the tests no longer run. The test file for 2+2=5 is no longer run and colcon test passes.

Details

  • OS: Ubuntu 22
  • ROS Distribution: humble
  • My environment had python packages installed the user environment, including setuptools 73.0.1

Logs

ros2 pkg create --build-type ament_python my_test_python
colcon build --packages-up-to my_test_python
Starting >>> my_test_python
/home/ryan/.local/lib/python3.10/site-packages/setuptools/_distutils/dist.py:268: UserWarning: Unknown distribution option: 'tests_require'
 warnings.warn(msg)
--- stderr: my_test_python                   
/home/ryan/.local/lib/python3.10/site-packages/setuptools/_distutils/dist.py:268: UserWarning: Unknown distribution option: 'tests_require'
 warnings.warn(msg)
---
Finished <<< my_test_python [1.01s]

Summary: 1 package finished [1.53s]

Implications

CI for ArduPIlot is no longer running our tests. This has allowed regressions to crop in our DDS interface. This is a huge issue for us. This linked issue is here: https://github.com/ArduPilot/ardupilot/issues/27925

Should we have a more clear warning that if you don't use the OS supplied setuptools, your tests won't be detected? I'd ideally want something to throw an error if you use the new setuptools with colcon.

Ryanf55 avatar Sep 07 '24 17:09 Ryanf55

Should we have a more clear warning that if you don't use the OS supplied setuptools, your tests won't be detected?

The problem is that that advice is not true in general, just sometimes. For instance, you were using the non-OS-supplied setuptools for quite a while before it broke. Further, colcon is a generic tool, not just for ROS, and thus whether things work or not depends on both the environment and the package.

That said, I could imagine adding an option to colcon to tell it to "fail" the tests if it found zero tests. We couldn't make this the default, since it is perfectly legitimate for a package to have no tests, but it might be a useful option for CI pipelines. That's just one thought, there might be others. Pinging @cottsay for additional thoughts.

clalancette avatar Sep 25 '24 10:09 clalancette

Hi, thanks for the report. I have a couple of thoughts.

  1. We are overdue to update the referenced documentation. The tests_require option was never official, and the modern practice (which is also not official) is to use an extras_require named test, tests, or testing. The change in setuptools that caused the change in behavior is that tests_require was pretty much dropped, and the mechanism we're using to read the python metadata no longer reports that field.
  2. In regard to how to behave when there are no tests, we're at the mercy of the underlying testing tool here. In fact, colcon doesn't have any idea how many tests there are, it just invokes the tool and checks the return code. There isn't any difference between a test run that actually ran no tests and a test run which didn't produce any JUnit/XUnit/etc result files for colcon test-result to find.

In this case, colcon no longer knew what Python test tool to use for this package because tests_require stopped working, so it defaulted to unittest instead of pytest. It's possible to write tests which can be invoked by either of those testing tools, but evidently these tests aren't able to be run by unttest.

Try adding this in place of tests_require:

    'extras_require': {
        'test': [
            'pytest',
        ],
    },

If that works as expected, we should update the documentation.

cottsay avatar Sep 26 '24 21:09 cottsay