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

Re-work colcon_core.command.get_prog_name #617

Closed cottsay closed 4 months ago

cottsay commented 7 months ago

This function's purpose is to handle these special cases of argv[0]:

This change enhances the path comparison to support normalization of the paths, Windows long path prefixes, and also the easy-install behavior on Windows where argv[0] has no extension.

Yet to be properly handled is invocation using python -c ...

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 83.33%. Comparing base (37dc1dd) to head (f5cb3c9). Report is 1 commits behind head on master.

Files Patch % Lines
colcon_core/command.py 80.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #617 +/- ## ========================================== + Coverage 83.27% 83.33% +0.06% ========================================== Files 66 66 Lines 3857 3865 +8 Branches 762 763 +1 ========================================== + Hits 3212 3221 +9 + Misses 556 555 -1 Partials 89 89 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cottsay commented 4 months ago

Do you think there's any value in unit-testing this...?

I think we could test at least some of it. Much of the behavior we're working around is specific to easy-install and Windows, but I might be able to specifically test the mechanics in some tests.

I'll add something to this PR.

cottsay commented 4 months ago

Do you think there's any value in unit-testing this...?

I'll add something to this PR.

Added a bunch of tests, and found a bug :upside_down_face:

cottsay commented 4 months ago

I had this at 100% test coverage locally, but it doesn't appear that codecov is correctly merging the results and reports a single missed line that is covered only on the Windows builds. I confirmed that the data that codecov is getting indicates coverage of the line, but the dashboard still reports it missing.

Obnoxious but not a problem.