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

Fixed code coverage on Windows #499

Closed
wants to merge 4 commits into from

Conversation

AttackTheDarkness
Copy link

Code coverage was broken on Windows, since file names were sometimes getting normalized with forward slashes, but sometimes weren't getting normalized. This made a bunch of matching logic break; the end result is that the array of covered files would always be empty. There was also a pretty serious problem when loading modules; a valid Windows path character (":") was getting used as a path delimiter, which was causing modules to get mapped to the user's drive letter rather than an actual path.

These changes make the code always deal with file names/paths in their native form; rather, they transform the configuration options from posix-style ("/") to Windows ("") before checking for matches.

Change summary:
Transform module paths/RegExps so the path matching logic works on Windows.
Use path.delimiter instead of ":" as a separator in module IDs, since ":" is a valid path character on Windows... All module paths in Windows were getting mapped as {drive letter} which wasn't super-useful.
Changed a check for absolute paths (check that first character is "/") to a more reliable cross-platform check for absolute paths (use path.isAbsolute).
Updated tests to expect different paths on different platforms.
Updated a unit test in HasteModuleLoader-requireMock-test.js to look for a Windows error string as well as the Unix string.
Removed unit tests which were expecting Windows-style paths to get normalized to posix-style paths (they don't, anymore).

Ryan Oriecuia added 3 commits September 11, 2015 14:30
Transform module paths/RegExps so the path matching logic works on Windows.
Use path.delimiter instead of ":" as a separator in module IDs, since ":" is a valid path character on Windows... All module paths in Windows were getting mapped as {drive letter} which wasn't super-useful.
Changed a check for absolute paths (check that first character is "/") to a more reliable cross-platform check for absolute paths (use path.isAbsolute).
Updated tests to expect different paths on different platforms.
Updated a unit test in HasteModuleLoader-requireMock-test.js to look for a Windows error string as well as the Unix string
We're using Node path utilities now, which will handle posix-to-Windows transformations on Windows, but not Windows-to-posix transfomations on Unix. Removed the unit tests that were making sure we were handling Windows-style paths.
@cpojer
Copy link
Member

cpojer commented Sep 11, 2015

thank you so much for the pull request. I'll take a look next week, but unfortunately don't have a windows machine to test on :(

@cpojer
Copy link
Member

cpojer commented Oct 14, 2015

This looks good. Would you mind rebasing it on master so I can pull it in? :)

Conflicts:
	src/TestRunner.js
@AttackTheDarkness
Copy link
Author

@cpojer it should be good to go now.

@cpojer
Copy link
Member

cpojer commented Oct 16, 2015

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1497189433935922/int_phab to review.

@cpojer
Copy link
Member

cpojer commented Oct 16, 2015

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1497189433935922/int_phab to review.

@ghost ghost closed this in db5bf02 Oct 16, 2015
@cpojer
Copy link
Member

cpojer commented Oct 16, 2015

Thanks @AttackTheDarkness – this will be in jest 0.6.0.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants