Add gatsby-node tests for plugin manifest #8598
Conversation
|
Minor thing here - this adds |
|
oh damn yeah sorry for that. i ll fix this soon |
| const fs = require(`fs`) | ||
| const { onPostBootstrap } = require(`../gatsby-node`) | ||
|
|
||
| jest.mock(`fs`) |
There was a problem hiding this comment.
These are hoisted to the top anyways, but would you mind adding it above line 1 (on line 1) for clarity?
There was a problem hiding this comment.
Also looks like you assign things below as jest spies. You can also do this
jest.mock(`fs`, () => {
return {
existsSync: jest.fn().mockImplementation(() => true),
writeFileSync: jest.fn()
};
});There was a problem hiding this comment.
hm i thought imports are first then mocks. i removed the skipped line if that was the issue although i am not sure i understood your comment correctly. I am brought the mocks up as well but didnt do as you suggested since then i need to mock statSync but only for second test. i hope this is what you are looking for :)
There was a problem hiding this comment.
Nope! It's more for ES module stuff, but it does hoist them to the top--I think it actually uses this babel plugin--so I prefer to just leave them at the top! Plus, at least to me, it clarifies things. I see a jest.mock('fs-extra') and then require('fs-extra') and I know it's mocked if that makes sense!
| theme_color: `#a2466c`, | ||
| display: `minimal-ui`, | ||
| }) | ||
| expect(fs.writeFileSync.mock.calls).toMatchSnapshot() |
There was a problem hiding this comment.
can you change this to something like:
const [filePath, contents] = fs.writeFileSync.writeFile.mock.calls[0]
expect(filePath).toEqual(path.join(`public`, `manifest.webmanifest`))
expect(contents).toMatchSnapshot()this test will fail on windows because of different path separator saved in snapshot
There was a problem hiding this comment.
done! sorry forgot windows.
* 'master' of github.com:gatsbyjs/gatsby: (66 commits) Fixed typos and made styling changes (gatsbyjs#8938) fixed typo in migrating-from-v0-to-v1.md (gatsbyjs#8921) Add https://developer.bitcoin.com/ to the Site Showcase (gatsbyjs#8931) Remove code block (gatsbyjs#8930) test(gatsby-plugin-feed): don't snapshot test platform specific (gatsbyjs#8836) test: add gatsby-node tests for plugin manifest (gatsbyjs#8598) fix(docs): Fix typo in using-gatsby-image.md (gatsbyjs#8889) docs: Correcting code diff highlights (gatsbyjs#8913) docs: e-commerce tutorial - allow cors request from react fetch (gatsbyjs#8660) docs: Updated code diff highlighting (gatsbyjs#8914) fix(www): Correct wrong entries in sites + starters (gatsbyjs#8888) fix(docs): consistent links to other tutorial parts (gatsbyjs#8860) feat(docs): Create "Using Fragments" article (gatsbyjs#8902) Use title syntax on markdown code blocks (gatsbyjs#8912) chore: use createContentDigest helper (gatsbyjs#8813) Updated hyperlink (gatsbyjs#8856) [v2/starter] Gatsby v2 Starter based on Bulma (gatsbyjs#8884) docs: update Web App Manifest compat (gatsbyjs#8891) docs: fix gatsby-source-graphql README links (gatsbyjs#8886) docs: fix diff highlighting in StaticQuery example (gatsbyjs#8892) ...
Related to #8371 #8580
I am not convinced gatsby-ssr tests are important to add. I mean besides adding some numbers to the overall percentage i am not sure a test would actually serve on something there even if adding it wouldn't hurt anything.
If you want i can add one.