-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-work colcon_core.command.get_prog_name #617
Conversation
This function's purpose is to handle these special cases of argv[0]: * Invoked using python -m ... * Invoked using a path to the executable even though the executable is on the PATH This change enhances the path comparison to support normalization of that path, 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think there's any value in unit-testing this with a bunch of mocks as a future enhancement or is this something that will only ever be proved out empirically?
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. |
Added a bunch of tests, and found a bug 🙃 |
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. |
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 ...