Fix tests on Windows #691
Merged
Fix tests on Windows #691
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
September 19, 2017 23:53
Jest takes an argument to specify which tests should be run. The argument is treated as a regular expression and matched against the filenames of potential tests. Unfortunately Jest does not normalize path separators to forward slashes when running this comparison, so (for example) `tests/unit` doesn't match anything on Windows. This change replaces the forward slash in Jest arguments with `[\\\\/]`. Ideally, that should just be `[\\/]` -- escaping the backslash so that it survives NPM's JSON parsing -- but unfortunately the Windows version of Jest parses its command line arguments differently from other platforms. Specifically, the Windows version of Jest processes backslashes one extra time. Fortunately having a redundant backslash is OK on Mac and Linux, so `[\\\\/]` ends up working on all platforms.
Adding `import 'chromedriver';` causes the module to register the path to `chromedriver` / `chromedriver.exe` automatically, fixing this error: ``` The ChromeDriver could not be found on the current PATH. Please download the latest version of the ChromeDriver from http://chromedriver.storage.googleapis.com/index.html and ensure it can be found on your PATH. at Error (native) ```
Contributor
rschamp
approved these changes
Oct 11, 2017
Contributor
rschamp
left a comment
rschamp
left a comment
There was a problem hiding this comment.
Those paths are hilariously weird, but LGTM. If it works it works. Thanks for grouping the steps under test:.
cwillisf
force-pushed
the
fix-tests-on-windows
branch
from
7d87f12 to
c99f187
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves
Jest-based integration tests currently do not run on Windows hosts.
Note that the integration tests don't run from Ubuntu-on-Windows without significant extra configuration. This change makes the tests work from a CMD prompt and maybe Cygwin.
Proposed Changes
This PR includes a few different changes which, combined, allow the tests to run on Windows. scratchfoundation/scratch-storage#22 should be treated as a prerequisite.
Reason for Changes
See the description of each commit below for a more thorough explanation. The short version:
[\\\\/]in the Jest test path arguments seems to be the only way to get the path to behave the same on both Windows and non-Windows platforms.import 'chromedriver';, Jest / Selenium can't findchromedriver.exeon Windows. This might also help other platforms...?npm run buildout of the integration test script and into the overall test script. In addition, I've changed the names of the test scripts as suggested by @rschamp./cc @fsih