Skip to content

Commit

Permalink
chore(*): Remove CSS false positives - Use newer JSDom version (#12719)
Browse files Browse the repository at this point in the history
## Description

Certain CSS styles have been failing to appear in Jest tests due to an outdated JSDom(and some it's dependencies, one being updated with bugfix in Feb 2019). This was causing false positives for `gatsby-image` tests for a while.

Nothing major, just some CSS props weren't appearing in tests when they technically should have been.

## Details

[My PR](#12468) is failing in the CI but was passing tests locally.

I had been running tests on the `gatsby-image` package on it's own, which uses `react-testing-library: ^5.0.0`, and it failed against the snapshot test. Updated snapshots failed when my PR ran on CircleCI. The main repo root yarn.lock file has `react-testing-library: ^5.2.1` locked down(retrieves 5.2.1 instead of 5.9.0, although is a v6 release now). 

Correcting this didn't help for passing the test suite. [`cssstyle: 1.2.0`](jsdom/cssstyle#74) fixes the [problems with certain styles](jsdom/jsdom#1965 (comment)) being [ignored](testing-library/react-testing-library#214. `transitionDelay`, and certain color values for `backgroundColor`, "red" works, but not "lightgray"). That however [needs `jsdom: ^12.0.0`](jsdom/jsdom@ed11465#diff-b9cfc7f2cdf78a7f4b91a753d10865a2), but [Jest refuses to update as support for Node <8 is dropped](jestjs/jest#8016 (comment))..

I've added a workaround until Jest supports a newer major version of JSDom. Using [`jest-environment-jsdom-fourteen`](https://github.com/ianschmitz/jest-environment-jsdom-fourteen) is the [advised approach](jestjs/jest#7122 (comment)) for the meantime, it'll only impact users running Jest test suite on Node 6, that should be fine for Gatsby? (Node 6 CI test seems to pass)

Only `gatsby-image` tests needed updates. [Node 6 will be EoL in May](https://github.com/nodejs/Release#release-schedule), no idea when Jest will bump JSDom from then, but we could wait a few months if you rather avoid the workaround?

## Related Issues

#12468 (Spotted it while working on this)
  • Loading branch information
polarathene authored and wardpeet committed Mar 27, 2019
1 parent 401401d commit 3b1fed2
Show file tree
Hide file tree
Showing 5 changed files with 250 additions and 9 deletions.
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,5 @@ module.exports = {
},
collectCoverageFrom: coverageDirs,
reporters: [`default`].concat(useCoverage ? `jest-junit` : []),
testEnvironment: `jest-environment-jsdom-fourteen`,
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"husky": "1.1.1",
"jest": "^24.0.0",
"jest-cli": "^24.0.0",
"jest-environment-jsdom-fourteen": "^0.1.0",
"jest-junit": "^6.1.0",
"jest-serializer-path": "^0.1.15",
"lerna": "^3.10.7",
Expand Down
12 changes: 6 additions & 6 deletions packages/gatsby-image/src/__tests__/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ exports[`<Img /> should render fixed size images 1`] = `
style="position: relative; overflow: hidden; display: inline; width: 100px; height: 100px;"
>
<div
style="width: 100px; opacity: 1; height: 100px;"
style="background-color: lightgray; width: 100px; opacity: 1; transition-delay: 0.25s; height: 100px;"
title="Title for the image"
/>
<img
alt=""
class="placeholder"
src="string_of_base64"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; opacity: 1; color: red;"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; object-fit: cover; object-position: center; opacity: 1; transition: opacity 0.5s; transition-delay: 0.25s; color: red;"
title="Title for the image"
/>
<picture>
Expand All @@ -29,7 +29,7 @@ exports[`<Img /> should render fixed size images 1`] = `
itemprop="item-prop-for-the-image"
src="test_image.jpg"
srcset="some srcSet"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; opacity: 0;"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; object-fit: cover; object-position: center; opacity: 0; transition: opacity 0.5s;"
title="Title for the image"
width="100"
/>
Expand All @@ -51,14 +51,14 @@ exports[`<Img /> should render fluid images 1`] = `
style="width: 100%; padding-bottom: 66.66666666666667%;"
/>
<div
style="position: absolute; top: 0px; bottom: 0px; opacity: 1; right: 0px; left: 0px;"
style="background-color: lightgray; position: absolute; top: 0px; bottom: 0px; opacity: 1; transition-delay: 0.25s; right: 0px; left: 0px;"
title="Title for the image"
/>
<img
alt=""
class="placeholder"
src="string_of_base64"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; opacity: 1; color: red;"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; object-fit: cover; object-position: center; opacity: 1; transition: opacity 0.5s; transition-delay: 0.25s; color: red;"
title="Title for the image"
/>
<picture>
Expand All @@ -74,7 +74,7 @@ exports[`<Img /> should render fluid images 1`] = `
sizes="(max-width: 600px) 100vw, 600px"
src="test_image.jpg"
srcset="some srcSet"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; opacity: 0;"
style="position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; object-fit: cover; object-position: center; opacity: 0; transition: opacity 0.5s;"
title="Title for the image"
/>
</picture>
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-image/src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe(`<Img />`, () => {
)
// No Intersection Observer in JSDOM, so placeholder img will be visible (opacity 1) by default
expect(placeholderImageTag.getAttribute(`style`)).toEqual(
`position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; opacity: 1; color: red;`
`position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; object-fit: cover; object-position: center; opacity: 1; transition: opacity 0.5s; transition-delay: 0.25s; color: red;`
)
expect(placeholderImageTag.getAttribute(`class`)).toEqual(`placeholder`)
})
Expand Down
Loading

0 comments on commit 3b1fed2

Please sign in to comment.