Fix getting display name and version when using windows registry #1697
Conversation
DonJayamanne
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1697 +/- ##
==========================================
+ Coverage 71.49% 71.96% +0.46%
==========================================
Files 277 277
Lines 13014 13023 +9
Branches 2344 2344
==========================================
+ Hits 9305 9372 +67
+ Misses 3574 3515 -59
- Partials 135 136 +1
Continue to review full report at Codecov.
|
DonJayamanne
changed the title
| public getPathVariableName() { | ||
| return this.isWindows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME; | ||
| } | ||
| public basename(pathValue: string, ext?: string): string{ |
There was a problem hiding this comment.
Typically it is not necessary to wrap over Path since behavior does not change across systems and it does not operate over 'real' things like actual files. We prob never going to mock it. So why not use path directly? We could eventually move getPathVariableName to IPlatform and drop this service althogether.
There was a problem hiding this comment.
I did want to mock this for one of the tests. I was relying on the tests to run on AppVeyor.
Mocking this makes it possible to run the tests on Travis as well, and not have to rely just on AppVeyor to run tests.
There was a problem hiding this comment.
Hm OK. Is behavior of basename is different across OS? Or did you mean path variable casing?
There was a problem hiding this comment.
Basename is different across OS, as path separator is different, it is either / or \
DonJayamanne
force-pushed
the
issue1660FixAppVeyorTests
branch
from
6f83b0e
to
9233d0d
Compare
Fixes #1660
Fixes #1703
This pull request: