search

colcon / colcon-clean

A colcon extension to clean package workspaces
http://colcon.readthedocs.io
Apache License 2.0
46 stars 4 forks source link

Improve --base-select and add --base-ignore #26

Closed ruffsl closed 2 years ago

ruffsl commented 2 years ago

Fix help message when printing default for --base-select To ensure all available base select options are displayed, including any external base handler extensions.

And add --base-ignore to ignore (default) base paths otherwise selected.

This allows uses to do things like cleaning all but a few ignored base paths, regardless of how many other base path and extension may be present.

ruffsl commented 2 years ago

For 57acdaaeeb67ddd8e8dadafd52b1d8498227f370 , I borrowed the test scaffolding from colcon-core, but I'm still not sure how it fixed the tests from erroring out from the expected ArgumentError. @cottsay , Do you know why this class based approach is needed.

cottsay commented 2 years ago

Do you know why this class based approach is needed.

The default behavior when argparse reacts to an ArgumentError is to call exit after printing the usage message. Unfortunately, this exits the tests as well. The _RaisingArgumentParser overrides the exit behavior to re-throw the exception that triggered the exit so that it can be caught instead. As it happens, Python 3.9 introduced a new kwarg to ArgumentParser to elicit this exact behavior, but we'll need to hang on to this implementation until we drop support for Python 3.8.

All that said, the class is designed to be instantiated in place of an argument parser and not as a test fixture, so I'm not sure how it's working in this code.

cottsay commented 2 years ago

I'm not sure how it's working in this code

I see - It's actually not working right now. test_suppress_argument_error isn't getting executed because the class name doesn't contain Test, I believe.

codecov-commenter commented 2 years ago

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #26 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 11 10 -1 Lines 263 266 +3 Branches 39 39 ========================================= + Hits 263 266 +3 ``` | [Impacted Files](https://codecov.io/gh/ruffsl/colcon-clean/pull/26?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruffin) | Coverage Δ | | |---|---|---| | [colcon\_clean/base\_handler/\_\_init\_\_.py](https://codecov.io/gh/ruffsl/colcon-clean/pull/26/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruffin#diff-Y29sY29uX2NsZWFuL2Jhc2VfaGFuZGxlci9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | | | [colcon\_clean/subverb/packages.py](https://codecov.io/gh/ruffsl/colcon-clean/pull/26/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruffin#diff-Y29sY29uX2NsZWFuL3N1YnZlcmIvcGFja2FnZXMucHk=) | `100.00% <100.00%> (ø)` | | | [colcon\_clean/subverb/workspace.py](https://codecov.io/gh/ruffsl/colcon-clean/pull/26/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruffin#diff-Y29sY29uX2NsZWFuL3N1YnZlcmIvd29ya3NwYWNlLnB5) | `100.00% <100.00%> (ø)` | | | [colcon\_clean/\_\_init\_\_.py](https://codecov.io/gh/ruffsl/colcon-clean/pull/26/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruffin#diff-Y29sY29uX2NsZWFuL19faW5pdF9fLnB5) | | | 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=Ruffin). 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=Ruffin)

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

ruffsl commented 2 years ago

Ah, ok. I fixed and simplified the monkey patching to override the stock error method.