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

Automated Testing: Unskip tests blocked by Enzyme support of React 16.3 #7005

Closed
aduth opened this issue May 29, 2018 · 9 comments
Closed

Automated Testing: Unskip tests blocked by Enzyme support of React 16.3 #7005

aduth opened this issue May 29, 2018 · 9 comments
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@aduth
Copy link
Member

aduth commented May 29, 2018

Previously: https://github.com/WordPress/gutenberg/pull/6527/files#r185974677

We currently skip tests which rely on React.forwardRef because they are not supported yet by Enzyme:

// Disable reason: This test is desirable, but unsupported by Enzyme in
// the current version, as it depends on features new to React in 16.3.0.
//
// eslint-disable-next-line jest/no-disabled-tests
describe.skip( 'ref forwarding', () => {
it( 'should enable access to DOM element', () => {
const ref = createRef();
mount( <ButtonWithForwardedRef ref={ ref } /> );
expect( ref.current.nodeName ).toBe( 'button' );
} );
} );

This is tracked by the Enzyme project at enzymejs/enzyme#1604

Once the above issue is closed and Enzyme produces a new release introducing support for React.forwardRef, we should remove the workarounds introduced by #6527 .

Open questions:

  • What else do we use from React 16.3 that isn't supported by Enzyme that will need to be updated?
  • What components other than Button have tests which need to be updated?
@aduth aduth added [Type] Task Issues or PRs that have been broken down into an individual action to take Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Status] Blocked Used to indicate that a current effort isn't able to move forward labels May 29, 2018
@gziolo
Copy link
Member

gziolo commented May 29, 2018

I just wanted to add that all tests that we skip currently are blocked by Enzyme's upgrade.

@gziolo gziolo removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Jul 13, 2018
@gziolo
Copy link
Member

gziolo commented Jul 13, 2018

This is how similar issues were fixed by @nerrad using React.TestUtils: #7835, #7834. We should go this route instead of waiting for enzyme to be upgraded.

@nerrad
Copy link
Contributor

nerrad commented Jul 13, 2018

Ya although its a pain, the more you work react-test-utils, and react-dom/test-utilities the less of a hassle it becomes. Definitely more reliable. It's kind of odd that the official react docs still suggest using enzyme.

@nerrad
Copy link
Contributor

nerrad commented Jul 13, 2018

Also worth noting that under the hood enzyme uses the various react test utilities.

@aduth
Copy link
Member Author

aduth commented Jul 18, 2018

Can we make sure to cross-reference pull requests meant to be addressing (or partially addressing) this issue.

@nerrad
Copy link
Contributor

nerrad commented Jul 18, 2018

As far as I know there's only one skipped (unit) test now.

@aduth
Copy link
Member Author

aduth commented Jul 18, 2018

Which pull requests are those? I'd like to keep record here for historical purposes.

@nerrad
Copy link
Contributor

nerrad commented Jul 18, 2018

The pulls I fixed skipped tests related to this were:

The pulls I converted away from enzyme (because they failed in #7557) are:

@aduth
Copy link
Member Author

aduth commented Aug 9, 2018

We no longer use Enzyme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

3 participants