Update colcon_core.command for generalized usage #651
Conversation
These non-breaking API changes are helpful for creating other CLIs based on colcon's extension model.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #651 +/- ##
==========================================
+ Coverage 83.24% 83.27% +0.03%
==========================================
Files 66 66
Lines 3849 3856 +7
Branches 760 762 +2
==========================================
+ Hits 3204 3211 +7
Misses 556 556
Partials 89 89 ☔ View full report in Codecov by Sentry. |
| 'environment variable' in str(e.value) | ||
| assert load.call_count == 0 | ||
| finally: | ||
| override_blocklist_variable(None) |
There was a problem hiding this comment.
This is what "resets" the blocklist variable and is responsible for the expected state difference between the test on https://github.com/colcon/colcon-core/pull/651/files#diff-307e796995136c87cf856ffb9123037838b3cc0a0e20838e2346f7f2a59784e0R155-R158 and the test on https://github.com/colcon/colcon-core/pull/651/files#diff-307e796995136c87cf856ffb9123037838b3cc0a0e20838e2346f7f2a59784e0R174-R180 correct?
There was a problem hiding this comment.
Precisely. First 2 checks makes sure that the override actually switched what variable the extension point subsystem is looking at, and the last check makes sure that the "reset to default" behavior works as expected. Also, because this changes globals, we don't want this test run to mess up other tests, so best to return it to the defaults.
I wanted to use a patch to "sandbox" this test a little more, but I'm pretty sure that the override will simply replace the mock.
These non-breaking API changes are helpful for creating other CLIs based on colcon's extension model.