Skip to content

Conversation

@DSchau
Copy link
Contributor

@DSchau DSchau commented Sep 20, 2018

This updates the internal gatsby version in one of the tests (path
prefix) so that the gatsby serve command now has the option
--prefix-paths which is required for the test to pass. To the best of
my knowledge, the --copy-all behavior of gatsby-dev-cli isn't meaningful
here, because it doesn't copy over the .bin executables, so the local
package.json versions are still used for some things.

Also note, for the chmod change, check out this test run. I've seen that happen locally after running gatsby-dev-cli, and that's the fix that's worked for me!

This updates the internal gatsby version in one of the tests (path
prefix) so that the `gatsby serve` command now has the option
`--prefix-paths` which is required for the test to pass. To the best of
my knowledge, the --copy-all behavior of gatsby-dev-cli isn't meaningful
here, because it doesn't copy over the .bin executables, so the local
package.json versions are still used for some things.

Also note, for the chmod change, check out [this test run](https://circleci.com/gh/gatsbyjs/gatsby/1410). I've seen that happen locally after running gatsby-dev-cli, and that's the fix that's worked for me!
gatsby-dev --set-path-to-repo $GATSBY_PATH &&
gatsby-dev --scan-once --copy-all && # copies _all_ files in gatsby/packages
gatsby-dev --scan-once --copy-all --quiet && # copies _all_ files in gatsby/packages
sudo chmod +x ./node_modules/.bin/gatsby && # this is sometimes necessary to ensure executable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I should maybe make this change at the gatsby-dev-cli level, instead, but this works for now.

Does anyone else run into this? I can replicate it pretty reliably on my local machine 🙃

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh really? I've never hit that problem — I thought NPM took care of that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's replicated in that test I linked to too to the best of my knowledge. Weird stuff 😱

@DSchau DSchau requested a review from pieh September 20, 2018 16:08
"dependencies": {
"cypress": "^3.1.0",
"gatsby": "next",
"gatsby": "^2.0.6",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be latest 🙃

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just hit permission problem with some of my tests, so let's try this out

@pieh pieh merged commit 2e21691 into gatsbyjs:master Sep 21, 2018
@DSchau DSchau deleted the e2e/path-prefix branch September 21, 2018 14:29
@DSchau
Copy link
Contributor Author

DSchau commented Sep 21, 2018

@pieh sounds good, thanks for the merge!

pieh added a commit that referenced this pull request Sep 21, 2018
oorestisime pushed a commit to oorestisime/gatsby that referenced this pull request Sep 28, 2018
…tsbyjs#8362)

* test: fix failing path prefix test by updating gatsby/cli version

This updates the internal gatsby version in one of the tests (path
prefix) so that the `gatsby serve` command now has the option
`--prefix-paths` which is required for the test to pass. To the best of
my knowledge, the --copy-all behavior of gatsby-dev-cli isn't meaningful
here, because it doesn't copy over the .bin executables, so the local
package.json versions are still used for some things.

Also note, for the chmod change, check out [this test run](https://circleci.com/gh/gatsbyjs/gatsby/1410). I've seen that happen locally after running gatsby-dev-cli, and that's the fix that's worked for me!

* ci: don't hardcode gatsby path
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.

3 participants