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

chore(gatsby-plugin-preload-fonts): Use createContentDigest helper #20155

Conversation

muescha
Copy link
Contributor

@muescha muescha commented Dec 16, 2019

Description

use createContentDigest

updated tests

Documentation

Related Issues

#8805 feat: update gatsby plugins to use new createContentDigest helper

@muescha muescha requested a review from a team as a code owner December 16, 2019 18:05
@muescha muescha changed the title chore(gatsby-theme-notes): Use createContentDigest helper chore(gatsby-plugin-preload-fonts): Use createContentDigest helper Dec 16, 2019
Copy link
Contributor

@freiksenet freiksenet left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! This looks fine, but unit tests are failing for node8.

@muescha
Copy link
Contributor Author

muescha commented Dec 18, 2019

need help

@freiksenet i need help to fix this

the problem with both failing test is that the new hashes are different on my local machine

steps i have done:

run test on Node 12

run test locally on v12.13.1 without errors

run test on Node 8

i and i also switched to Node v8.17:

$ nvm list
->      v8.17.0
       v10.15.3
       v12.13.1
        v13.0.1
        v13.1.0
         system
default -> v12.13.1
node -> stable (-> v13.1.0) (default)
stable -> 13.1 (-> v13.1.0) (default)
iojs -> N/A (default)
unstable -> N/A (default)
lts/* -> lts/erbium (-> N/A)
lts/argon -> v4.9.1 (-> N/A)
lts/boron -> v6.17.1 (-> N/A)
lts/carbon -> v8.17.0
lts/dubnium -> v10.18.0 (-> N/A)
lts/erbium -> v12.14.0 (-> N/A)
[09:29:49] muescha@Michaels-MacBook-Pro-2:~/Work/gatsby/github/gatsby/packages/gatsby-plugin-preload-fonts 

but still the tests passing with the new hashes:

$ jest

 RUNS  src/__tests__/fetch-routes.test.js

 RUNS  src/__tests__/fetch-routes.test.js

 RUNS  src/__tests__/fetch-routes.test.js
 PASS  src/__tests__/utils.test.js

 RUNS  src/__tests__/fetch-routes.test.js

 RUNS  src/__tests__/fetch-routes.test.js
 PASS  src/__tests__/cache.test.js

 RUNS  src/__tests__/fetch-routes.test.js
 PASS  src/__tests__/gatsby-ssr.test.js

 RUNS  src/__tests__/fetch-routes.test.js
 PASS  src/__tests__/index.test.js
  ● Console

    console.log src/prepare/logger.js:49
      undefined

 PASS  src/__tests__/logger.test.js
 PASS  src/__tests__/fetch-routes.test.js

Test Suites: 6 passed, 6 total
Tests:       20 passed, 20 total
Snapshots:   6 passed, 6 total
Time:        3.237s
Ran all test suites.

check master tests

  • the test on master branch (without my changes) also run without errors

@muescha muescha added the help wanted Issue with a clear description that the community can help with. label Dec 18, 2019
@pieh
Copy link
Contributor

pieh commented Dec 18, 2019

That's very weird, I'll take a look at this

@pieh
Copy link
Contributor

pieh commented Dec 18, 2019

Can you run yarn bootstrap in root of repository and try rerun tests again? I suspect that you might not have built gatsby-core-utils recently and if your built version is from before #19832 it could explain difference in generated hashes

(I get the same results as on CI locally)

As for node 8 vs other node versions - we just run this first and don't run more (potentially long) checks if it fails, so it doesn't mean that it's node version related issue

@muescha
Copy link
Contributor Author

muescha commented Dec 18, 2019

Ah ok. That was my idea with the node version

Good catch. That maybe is different. I will rerun bootrap again later and try it again

@muescha
Copy link
Contributor Author

muescha commented Dec 18, 2019

bootstrapped and now my local hashes the same as CI - now test run successful

thanks @pieh for help

@pieh pieh dismissed freiksenet’s stale review December 18, 2019 15:18

failing tests are fixed

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.

Thanks @muescha!

@muescha muescha removed the help wanted Issue with a clear description that the community can help with. label Dec 18, 2019
@pieh pieh merged commit 9b2a3bf into gatsbyjs:master Dec 18, 2019
@muescha muescha deleted the muescha/refactor/createContentDigest-plugin-preload-fonts branch December 18, 2019 16:04
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