add function to detect and render helmet for use in snapshots #2104

jroebu14 · 2019-09-12T11:04:48Z

Overall change: Adds functionality to shouldMatchSnapshot to include html and head tags if react-helmet was used inside of component. This enables us to see things like meta tags and scripts in snapshots.

Code changes:

Adds new function called renderWithHelmet that is a wrapper around the @testing-library/react render method.

An example using the Chartbeat component in Simorgh

const CanonicalChartbeatBeacon = ({ chartbeatConfig, chartbeatSource }) => {
  useEffect(() => {
    return () => {
      if (typeof window !== 'undefined' && window.pSUPERFLY) {
        /*
          This function is always called to update config values on page changes
          https://chartbeat.zendesk.com/hc/en-us/articles/210271287-Handling-virtual-page-changes
        */
        window.pSUPERFLY.virtualPage(chartbeatConfig);
      }
    };
  }, [chartbeatConfig]);

  return (
    <>
      <Helmet>
        <script async type="text/javascript">
          {`
        (function(){
          var _sf_async_config = window._sf_async_config = (window._sf_async_config || {});
          var config = ${JSON.stringify(chartbeatConfig)};
          for (var key in config) {
            _sf_async_config[key] = config[key];
          }
        })();
      `}
        </script>
        <script async type="text/javascript" src={chartbeatSource} />
      </Helmet>
      <h1>Hello I should be in the body</h1>
    </>
  );
};

would then render

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`CanonicalChartbeatAnalytics should return the helmet wrapper with the script snippet 1`] = `
<html>
  <head>
    <script
      async="true"
      type="text/javascript"
    >
      
        (function(){
          var _sf_async_config = window._sf_async_config = (window._sf_async_config || {});
          var config = {"domain":"test-domain","type":"article","sections":"section1 section2","chartbeatUID":1111,"virtualReferrer":null,"useCanonical":true,"title":"This is an article","uid":123};
          for (var key in config) {
            _sf_async_config[key] = config[key];
          }
        })();
      
    </script>
    <script
      async="true"
      src="//chartbeat.js"
      type="text/javascript"
    />
  </head>
  <body>
    <div>
      <h1>
        Hello I should be in the body
      </h1>
    </div>
  </body>
</html>
`;

I have assigned myself to this PR and the corresponding issues
Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
This PR requires manual testing

dr3

LGTM, just some simple snapshot tests would be nice IMO as its more complicated

packages/utilities/psammead-test-helpers/src/index.js

…t-snapshots

packages/utilities/psammead-test-helpers/src/index.js

dr3 · 2019-09-12T16:09:43Z

packages/utilities/psammead-test-helpers/src/index.js

 export const shouldMatchSnapshot = (title, component) => {
-  it(title, () => {
+  it(title, async () => {
    // select the first child to remove the pointless wrapping div from snapshots


This happens now :(

Are you worried about the async it callback? It won't affect the current test helper API.

Soz i meant select the first child to remove the pointless wrapping div from snapshots happens now. We no longer remove the pointless wrapping div

We still do when rendering components without helmet. I'll change the comments and var names.

dr3 · 2019-09-12T16:09:59Z

packages/utilities/psammead-test-helpers/__snapshots__/index.test.jsx.snap

+      src="test.js"
+    />
+  </head>
+  <body>


Any easy way to remove the body when not needed?

I'll look into it. Was worried about adding more complexity to an already complex looking function for not much value but it should be straightforward

I tried removing the body but that stopped other tests from rendering things in the body. I guess it breaks the renderer because it can't find the body element's div to put stuff in. I also tried this with removing just the div and it had the same effect :(

pjlee11 · 2019-09-12T17:48:18Z

Jenkins is failing because this updates packages/utilities/moment-timezone-include/package-lock.json but doesn't bump the package version and changelog

…e-lock.json not sure why this was different

…t-snapshots

dr3

Soz for my confusion, looks great :)

add function to detect and render helmet for use in snapshots

d46ae43

jroebu14 force-pushed the helmet-snapshots branch from 5949d78 to d46ae43 Compare September 12, 2019 11:06

Jonathan Roebuck added 2 commits September 12, 2019 12:40

Bump package version for psammead-test-helpers

db0c836

Update changelog for psammead-test-helpers

140b90c

jroebu14 force-pushed the helmet-snapshots branch from 0a04b57 to 140b90c Compare September 12, 2019 11:42

jroebu14 requested a review from dr3 September 12, 2019 11:45

jroebu14 self-assigned this Sep 12, 2019

jroebu14 added ws-media The World Service media stream shared-components labels Sep 12, 2019

jroebu14 requested a review from sareh September 12, 2019 11:48

jroebu14 marked this pull request as ready for review September 12, 2019 11:48

jroebu14 requested review from jamesbhobbs and j-pendlebury as code owners September 12, 2019 11:48

jroebu14 requested a review from chris-hinds September 12, 2019 11:53

dr3 suggested changes Sep 12, 2019

View reviewed changes

packages/utilities/psammead-test-helpers/src/index.js Outdated Show resolved Hide resolved

Jonathan Roebuck added 8 commits September 12, 2019 14:08

add snapshot tests in psammead-test-helper

733aaa2

use correct test case titles

bf4cebe

better variable name

28aed67

add more testing around snapshots

5e717f0

Merge branch 'latest' into helmet-snapshots

58e374d

Merge branch 'latest' into helmet-snapshots

9b3cf45

fix duplicate head content bug in psammead-test-helpers

98d5bac

Merge branch 'helmet-snapshots' of github.com:bbc/psammead into helme…

e86c3e7

…t-snapshots

chris-hinds approved these changes Sep 12, 2019

View reviewed changes

update package-locks

0234a49

ryanmccombe reviewed Sep 12, 2019

View reviewed changes

packages/utilities/psammead-test-helpers/src/index.js Outdated Show resolved Hide resolved

ryanmccombe approved these changes Sep 12, 2019

View reviewed changes

dr3 suggested changes Sep 12, 2019

View reviewed changes

update code comments in psammead-test-helpers

da16246

Jonathan Roebuck added 3 commits September 12, 2019 18:49

git checkout latest packages/utilities/moment-timezone-include/packag…

49d8606

…e-lock.json not sure why this was different

use correct meta attributes

b72bab5

Merge branch 'latest' into helmet-snapshots

fa62362

jroebu14 mentioned this pull request Sep 13, 2019

Create test helper function to render react-helmet html and head for snapshots bbc/simorgh#3694

Closed

3 tasks

Jonathan Roebuck added 4 commits September 13, 2019 14:42

add tests for renderWithHelmet

43b9a57

Merge branch 'helmet-snapshots' of github.com:bbc/psammead into helme…

90ca65f

…t-snapshots

add and reorder tests

7b2bad9

add better test case titles

393bfce

dr3 approved these changes Sep 13, 2019

View reviewed changes

jroebu14 merged commit 422bcd7 into latest Sep 13, 2019

jroebu14 deleted the helmet-snapshots branch September 13, 2019 14:25

bbc-news-frameworks mentioned this pull request Sep 19, 2019

Talos - Bump @bbc/psammead-amp-geo, @bbc/psammead-brand, @bbc/psammead-caption... #2191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add function to detect and render helmet for use in snapshots #2104

add function to detect and render helmet for use in snapshots #2104

jroebu14 commented Sep 12, 2019 •

edited

Loading

dr3 left a comment

dr3 Sep 12, 2019

jroebu14 Sep 12, 2019

dr3 Sep 13, 2019 •

edited

Loading

jroebu14 Sep 13, 2019

dr3 Sep 12, 2019

jroebu14 Sep 12, 2019

jroebu14 Sep 12, 2019

pjlee11 commented Sep 12, 2019

dr3 left a comment

add function to detect and render helmet for use in snapshots #2104

add function to detect and render helmet for use in snapshots #2104

Conversation

jroebu14 commented Sep 12, 2019 • edited Loading

dr3 left a comment

Choose a reason for hiding this comment

dr3 Sep 12, 2019

Choose a reason for hiding this comment

jroebu14 Sep 12, 2019

Choose a reason for hiding this comment

dr3 Sep 13, 2019 • edited Loading

Choose a reason for hiding this comment

jroebu14 Sep 13, 2019

Choose a reason for hiding this comment

dr3 Sep 12, 2019

Choose a reason for hiding this comment

jroebu14 Sep 12, 2019

Choose a reason for hiding this comment

jroebu14 Sep 12, 2019

Choose a reason for hiding this comment

pjlee11 commented Sep 12, 2019

dr3 left a comment

Choose a reason for hiding this comment

jroebu14 commented Sep 12, 2019 •

edited

Loading

dr3 Sep 13, 2019 •

edited

Loading