colcon / colcon-core

Command line tool to build sets of software packages
http://colcon.readthedocs.io
Apache License 2.0
103 stars 46 forks source link

Revert "Deprecate colcon_core.task.python.test.has_test_dependency()" #548

Closed cottsay closed 1 year ago

cottsay commented 1 year ago

It turns out that this mechanism is used to communicate the Python-type dependencies from the setuptools options when processing ament_python packages, which otherwise have ROS-type dependencies. So while we're seeing python3-pytest in the test dependency list, the pure python code here looks for the pure python dependency pytest.

Since this doesn't actually save us much time and is clearly still needed, I'll take the same approach in the Python project code as was taken in ament_python so that we can all use the same Python testing task.

This reverts #536.

codecov[bot] commented 1 year ago

Codecov Report

Base: 81.75% // Head: 82.01% // Increases project coverage by +0.25% :tada:

Coverage data is based on head (970a90d) compared to base (a6c04e0). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #548 +/- ## ========================================== + Coverage 81.75% 82.01% +0.25% ========================================== Files 62 62 Lines 3645 3642 -3 Branches 705 705 ========================================== + Hits 2980 2987 +7 + Misses 612 603 -9 + Partials 53 52 -1 ``` | [Impacted Files](https://codecov.io/gh/colcon/colcon-core/pull/548?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon) | Coverage Δ | | |---|---|---| | [colcon\_core/task/python/test/\_\_init\_\_.py](https://codecov.io/gh/colcon/colcon-core/pull/548?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon#diff-Y29sY29uX2NvcmUvdGFzay9weXRob24vdGVzdC9fX2luaXRfXy5weQ==) | `32.71% <ø> (+5.18%)` | :arrow_up: | | [colcon\_core/task/python/test/pytest.py](https://codecov.io/gh/colcon/colcon-core/pull/548?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon#diff-Y29sY29uX2NvcmUvdGFzay9weXRob24vdGVzdC9weXRlc3QucHk=) | `28.75% <100.00%> (+0.35%)` | :arrow_up: | | [colcon\_core/task/python/\_\_init\_\_.py](https://codecov.io/gh/colcon/colcon-core/pull/548?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon#diff-Y29sY29uX2NvcmUvdGFzay9weXRob24vX19pbml0X18ucHk=) | `33.33% <0.00%> (+9.52%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

cottsay commented 1 year ago

is there any way we can add a test to catch this situation?

We can't test for it here because colcon-ros is where then shenanigans are happening. Even then, we don't have a regular infrastructure CI job where the changes would have been tested together.