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

Convert snapshots to use react-test-renderer/shallow (core-blocks/more/test/edit.js) #7756

Merged

Conversation

nerrad
Copy link
Contributor

@nerrad nerrad commented Jul 6, 2018

Note: with react-test-renderer, the current version of the jest pretty-format module does not handle pretty names for React.Fragment and React.forwardRef. Thus the snapshot results in <Undefined> for related components in the rendered tree for the formats.

I've confirmed that updating the react element serializer in the pretty-format package (via manually editing it in node_modules) to the latest as it is here fixes it. I'm not quite sure what is setting up this dependency so some assistance will be needed to get the related modules updated so the new pretty-format serializer is being used.

I did not update the snapshot in this pull yet until that is resolved.

Tests with snapshots to update when Jest has been updated:

  • core-blocks/more/test/edit.js
  • editor/components/block-switcher/test/multi-blocks-switcher.js
  • editor/components/default-block-appender/test/index.js
  • editor/components/post-saved-state/test/index.js

@nerrad nerrad self-assigned this Jul 6, 2018
@nerrad nerrad added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Jul 6, 2018
@nerrad nerrad requested a review from gziolo July 6, 2018 19:45
@nerrad
Copy link
Contributor Author

nerrad commented Jul 6, 2018

With WordPress/packages#136 and #7556, it looks like jest will get updated. Once those land I'll pull the changes into this branch and then it should fix this.

@nerrad
Copy link
Contributor Author

nerrad commented Jul 8, 2018

There's also a similar failing snapshot test for editor/components/block-switcher/test/multi-blocks-switcher.js that I will regenerate in this pull as well. I'm going to start a checklist of snapshots to regenerate in the description for the pull.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I see the following in Jest docs https://jestjs.io/docs/en/snapshot-testing:

import React from 'react';
import Link from '../Link.react';
import renderer from 'react-test-renderer';

it('renders correctly', () => {
  const tree = renderer
    .create(<Link page="http://www.facebook.com">Facebook</Link>)
    .toJSON();
  expect(tree).toMatchSnapshot();
});

Can we follow it?

Jest 23.3 is now included in Gutenberg.

@nerrad
Copy link
Contributor Author

nerrad commented Jul 12, 2018

Can we follow it?

I think it depends, there may be some cases where we want to do toTree() because that is necessary to perform the snapshot on user created components, whereas toJSON() only outputs native rendered components.

[ update ]
However, the snapshots in here are using react-test-renderer/shallow (getRenderOutput) which more closely align with enzyme.shallow which were used originally. As we see with the now passing tests after bringing in the updated Jest package from master, the snapshots actually match what's saved (which was a bit of a surprise but still nice :) )

Note: with react-test-renderer, the current version of the jest pretty-format module does not handle pretty names for `React.Fragment` and `React.forwardRef`.  Thus the snapshot results in `<Undefined>`.
@nerrad nerrad force-pushed the update/convert-enzyme-tests-to-react-test-renderer-edit.js branch from e67e011 to cee0bea Compare July 12, 2018 17:08
@nerrad
Copy link
Contributor Author

nerrad commented Jul 12, 2018

So if travis reports the same as what I saw locally, simply updating master on this branch fixes things (because the snapshots continue to match). The snapshots mentioned in the description will need updated in #7557 once this lands there.

@nerrad nerrad requested a review from gziolo July 12, 2018 17:31
@gziolo
Copy link
Member

gziolo commented Jul 13, 2018

Oh, this is an interesting difference between those two. I always assumed that this sample code from Jest docs operates on shallow rendered output.

@gziolo gziolo added this to the 3.3 milestone Jul 13, 2018
@gziolo gziolo merged commit 843ba97 into master Jul 13, 2018
@gziolo gziolo deleted the update/convert-enzyme-tests-to-react-test-renderer-edit.js branch July 13, 2018 07:11
@gziolo
Copy link
Member

gziolo commented Jul 13, 2018

How about other files? Are they fixed in different PRs?

  • editor/components/block-switcher/test/multi-blocks-switcher.js
  • editor/components/default-block-appender/test/index.js
  • editor/components/post-saved-state/test/index.js

@nerrad
Copy link
Contributor Author

nerrad commented Jul 13, 2018

I think those are snapshots that actually failed in #7557 so its likely they'll need updated there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants