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

fix(gatsby-plugin-page-creator): ensure that __tests__ directory is actually ignored #9720

Merged
merged 8 commits into from
Nov 6, 2018

Conversation

DSchau
Copy link
Contributor

@DSchau DSchau commented Nov 5, 2018

This (simple) PR fixes a bug I was noticing in #9629, specifically that
tests/*.js files were being included in the bundle.

This is because the isTestFile function in gatsby-plugin-page-creator is
passed the file name, not the file name with the folder (tests in
this case).

So, this PR does the following:

  • Adds a fallback check to check the base dir (which could be tests)
  • (Also) ignores typescript files as part of this check, since this isn't readily apparent how it would be configurable to ignore them currently

Note: I could've solved this by also passing the raw path (not system path parsed) but wanted to avoid any Windows idiosyncrasies just in case

This (simple) PR fixes a bug I was noticing in gatsbyjs#9629, specifically that
__tests__/*.js files _were_ being included in the bundle.

This is because the isTestFile function in gatsby-plugin-page-creator is
passed the file name, not the file name _with_ the folder (__tests__ in
this case).

I added a second check to check the dir, but could also just implement
this check with the raw file path, as well
@@ -10,28 +10,26 @@ describe(`JavaScript page creator`, () => {
{ path: `somedir/dir2/test1.js` },
]

expect(validFiles.filter(file => validatePath(file.path)).length).toEqual(validFiles.length)
expect(validFiles.filter(file => validatePath(file.path))).toEqual(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also removed all the .length checks. It's a lot safer to validate the actual array shape rather than the length of the array, even though there weren't any changes required here!

e.g. if one of the validFiles was failing and one of the invalid files was passing, we could still get the expected length

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.

🙏

@pieh pieh changed the title fix: ensure that __tests__ directory is actually ignored fix(gatsby-plugin-page-creator): ensure that __tests__ directory is actually ignored Nov 6, 2018
@pieh
Copy link
Contributor

pieh commented Nov 6, 2018

e2e tests failing because of new dependency not being handled, so merging this

@pieh pieh merged commit a6d17d8 into gatsbyjs:master Nov 6, 2018
lipis added a commit to lipis/gatsby that referenced this pull request Nov 6, 2018
* 'master' of github.com:gatsbyjs/gatsby: (63 commits)
  Update how-to-contribute.md to mention the style guide when writing blogs/tutorials (gatsbyjs#9742)
  Added  Tylermcginnis website (gatsbyjs#9619)
  Fix grammar and punctuation (gatsbyjs#9498)
  Fix typo of plugin authoring (gatsbyjs#9737)
  Authentication tutorial - small fixes (gatsbyjs#9738)
  chore: move run-sift (gatsbyjs#9549)
  docs: fix minor typo (gatsbyjs#9730)
  chore(release): Publish
  fix(gatsby-plugin-page-creator): ensure that __tests__ directory is actually ignored (gatsbyjs#9720)
  fix: revert admin redirect (gatsbyjs#9728)
  fix: adjust page order to make nested matchPaths work (gatsbyjs#9719)
  feat(gatsby-plugin-sharp): cache base64 if possible (gatsbyjs#9059)
  chore(release): Publish
  fix(gatsby-plugin-offline): Serve the offline shell for short URLs + use no-cors for external resources (gatsbyjs#9679)
  chore(release): Publish
  fix: ensure babel-preset-gatsby can be used with unit tests (gatsbyjs#9629)
  feat(www): Filter posts by date (gatsbyjs#9400)
  fix(blog): azure blog post url date (gatsbyjs#9718)
  feat(blog): Add post on publishing to Azure (gatsbyjs#8868)
  Emphasize importance of promise return on source-plugin docs (gatsbyjs#9650)
  ...
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
…ctually ignored (gatsbyjs#9720)

* fix: ensure that __tests__ directory is actually ignored

This (simple) PR fixes a bug I was noticing in gatsbyjs#9629, specifically that
__tests__/*.js files _were_ being included in the bundle.

This is because the isTestFile function in gatsby-plugin-page-creator is
passed the file name, not the file name _with_ the folder (__tests__ in
this case).

I added a second check to check the dir, but could also just implement
this check with the raw file path, as well

* chore: use back ticks

* fix: account for nested directories and switch to micromatch

* chore: split to test windows path matching

* chore: run linter

* chore: add windows path matching test (micromatch supports natively)

* fix: pass single string to isMatch, instead of array

* Revert "fix: pass single string to isMatch, instead of array"

This reverts commit dbc2ffe.
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.

2 participants