Skip to content
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

Ensure case is normalized on Windows when comparing paths from XML coverage report #100

Merged
merged 1 commit into from May 10, 2019
Merged

Conversation

nicoddemus
Copy link
Contributor

This fixes a subtle problem on Windows, which is case-preserving but case-insensitive.

Depending from where the user executed the coverage command to produce the XML file, the <sources> tag might contain a different case in the drive letter: w:\foo\bar vs W:\foo\bar (note the case of the first letter). On Windows both paths should be considered the same.

This patch calls the os.path.normcase function, which changes all paths to lower-case on Windows and is a noop on Linux.

I experienced this problem myself and with this patch this fixes the issue for me.

…verage report

This fixes a subtle problem on Windows, which is case-preserving but case-insensitive.
Depending from where the user executed the coverage command to produce the XML file,
the `<sources>` tag might contain a different case in the drive letter: `w:\foo\bar`
vs `W:\foo\bar` (note the case of the first letter). On Windows both paths should
be considered the same.

This patch calls the `os.path.normcase` function, which changes all paths
to lower-case on Windows and is a noop on Linux.

I experienced this problem myself and with this patch this fixes the issue
for me.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 99.139% when pulling 0fca8c5 on nicoddemus:normcase-windows into 901cb3f on Bachmann1234:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 99.139% when pulling 0fca8c5 on nicoddemus:normcase-windows into 901cb3f on Bachmann1234:master.

@Bachmann1234
Copy link
Owner

Missing line because my test suite does not test windows. Ill file an issue to add windows to travis

@Bachmann1234
Copy link
Owner

Bachmann1234 commented May 10, 2019

Thanks for the PR! Glad to see windows getting some love

@Bachmann1234 Bachmann1234 merged commit 6f0fc0b into Bachmann1234:master May 10, 2019
@Bachmann1234
Copy link
Owner

@nicoddemus nicoddemus deleted the normcase-windows branch May 10, 2019 11:41
@nicoddemus
Copy link
Contributor Author

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants