Find and use full path to percy-healthcheck script #67
Conversation
This file contains 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
Robdel12
reviewed
Mar 27, 2019
Robdel12
approved these changes
Mar 27, 2019
|
Shipped with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The main change here is getting away from trying to rely on
nodeto find the path to abinwe install, and instead try to compute the full path to thepercy-healthcheckscript ourselves, in a way similar to how we do that forpercy-agent.js.This change also adds some additional logging that should help in the case when the healthcheck fails.
I did not succeed in doing a magic
npm linkbefore calling the tests, so that in the package tests we can rely on@percy/cypressbeing installed, so I'm still falling back on the somewhat ugly option of manually copying stuff intonode_modulesbefore the tests run. Since this is just for development (for the tests in this package), I think that's ok.@Robdel12 @djones for review