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

Jest Preset: Major version upgrade for Jest in all packages #20766

Merged
merged 14 commits into from
Apr 9, 2020

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Mar 10, 2020

Description

This PR updated Jest to the last version. It also upgrades all related dependencies like enzyme or jsdom.

It was triggered by the issue discovered by @brentswisher in #20514. Related comments:

The Disabled component uses a MutationObserver which jest doesn't support. Using the unit test for the Disabled component as a template I added a stub of window.MutationObserver in beforeAll() and removed it in afterAll(). This appears to work but doesn't seem ideal as none of the other storybook tests need it. If there is a better way to do this feel free to point me in that direction.

As discussed on Slack yesterday, we will need to upgrade Jest to use the latest version of JSDom which supports MutationObserver.

TODO

  • fix failing unit tests
  • include CHANGELOG entries

How has this been tested?

Executed:
npm run test-unit
npm run test-e2e

There are still some unit tests failing ...

There is no observed impact on e2e tests.

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@gziolo gziolo added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Tool] WP Scripts /packages/scripts labels Mar 10, 2020
@gziolo gziolo requested review from getdave, nerrad and diegohaz March 10, 2020 11:55
@gziolo gziolo self-assigned this Mar 10, 2020
@gziolo
Copy link
Member Author

gziolo commented Mar 10, 2020

I still see some failing tests that need to be further investigated. It looks like all of them are related to DOM structure or interactions I would appreciate help from @diegohaz, @getdave or @nerrad. Would it help to land one of the PRs with react-testing-library integration first?

Summary of all failing tests
 FAIL  packages/components/src/sandbox/test/index.js
  ● should rerender with new emdeded content if html prop changes

    expect(jest.fn()).not.toHaveErrored(expected)

    Expected mock function not to be called but it was called with:
    ["Error: Uncaught [TypeError: Cannot read property 'body' of undefined]
        at reportException (/Users/gziolo/Projects/gutenberg/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:62:24)
        at notifyMutationObservers (/Users/gziolo/Projects/gutenberg/node_modules/jsdom/lib/jsdom/living/helpers/mutation-observers.js:166:
11)
        at /Users/gziolo/Projects/gutenberg/node_modules/jsdom/lib/jsdom/living/helpers/mutation-observers.js:133:5
        at processTicksAndRejections (internal/process/task_queues.js:97:5)", [TypeError: Cannot read property 'body' of undefined]]

      34 |      function assertExpectedCalls() {
      35 |              if ( spy.assertionsNumber === 0 && spy.mock.calls.length > 0 ) {
    > 36 |                      expect( console ).not[ matcherName ]();
         |                      ^
      37 |              }
      38 |      }
      39 | 

      at Object.assertExpectedCalls (packages/jest-console/src/index.js:36:4)

 FAIL  packages/components/src/navigable-container/test/tabbable.js
  ● TabbableContainer › should navigate by keypresses

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      88 |              ) {
      89 |                      const interaction = fireKeyDown( container, keyCode, shiftKey );
    > 90 |                      expect( currentIndex ).toBe( expectedActiveIndex );
         |                                             ^
      91 |                      expect( interaction.stopped ).toBe( expectedStop );
      92 |              }
      93 | 

      at assertKeyDown (packages/components/src/navigable-container/test/tabbable.js:90:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/tabbable.js:94:3)

  ● TabbableContainer › should navigate by keypresses and stop at edges

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      145 |             ) {
      146 |                     const interaction = fireKeyDown( container, keyCode, shiftKey );
    > 147 |                     expect( currentIndex ).toBe( expectedActiveIndex );
          |                                            ^
      148 |                     expect( interaction.stopped ).toBe( expectedStop );
      149 |             }
      150 | 

      at assertKeyDown (packages/components/src/navigable-container/test/tabbable.js:147:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/tabbable.js:151:3)

 FAIL  packages/components/src/navigable-container/test/menu.js
  ● NavigableMenu › vertical: should navigate by up and down

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      83 |              function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
      84 |                      const interaction = fireKeyDown( container, keyCode, false );
    > 85 |                      expect( currentIndex ).toBe( expectedActiveIndex );
         |                                             ^
      86 |                      expect( interaction.stopped ).toBe( expectedStop );
      87 |              }
      88 | 

      at assertKeyDown (packages/components/src/navigable-container/test/menu.js:85:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/menu.js:89:3)

  ● NavigableMenu › vertical: should navigate by up and down, and stop at edges

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      137 |             function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
      138 |                     const interaction = fireKeyDown( container, keyCode, false );
    > 139 |                     expect( currentIndex ).toBe( expectedActiveIndex );
          |                                            ^
      140 |                     expect( interaction.stopped ).toBe( expectedStop );
      141 |             }
      142 | 

      at assertKeyDown (packages/components/src/navigable-container/test/menu.js:139:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/menu.js:143:3)

  ● NavigableMenu › horizontal: should navigate by left and right

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      193 |             function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
      194 |                     const interaction = fireKeyDown( container, keyCode, false );
    > 195 |                     expect( currentIndex ).toBe( expectedActiveIndex );
          |                                            ^
      196 |                     expect( interaction.stopped ).toBe( expectedStop );
      197 |             }
      198 | 

      at assertKeyDown (packages/components/src/navigable-container/test/menu.js:195:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/menu.js:199:3)

  ● NavigableMenu › horizontal: should navigate by left and right, and stop at edges

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      247 |             function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
      248 |                     const interaction = fireKeyDown( container, keyCode, false );
    > 249 |                     expect( currentIndex ).toBe( expectedActiveIndex );
          |                                            ^
      250 |                     expect( interaction.stopped ).toBe( expectedStop );
      251 |             }
      252 | 

      at assertKeyDown (packages/components/src/navigable-container/test/menu.js:249:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/menu.js:253:3)

  ● NavigableMenu › both: should navigate by up/down and left/right

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      292 |             function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
      293 |                     const interaction = fireKeyDown( container, keyCode );
    > 294 |                     expect( currentIndex ).toBe( expectedActiveIndex );
          |                                            ^
      295 |                     expect( interaction.stopped ).toBe( expectedStop );
      296 |             }
      297 | 

      at assertKeyDown (packages/components/src/navigable-container/test/menu.js:294:27)
      at Object.<anonymous> (packages/components/src/navigable-container/test/menu.js:298:3)

 FAIL  packages/components/src/higher-order/with-focus-return/test/index.js
  ● withFocusReturn() › testing rendering and focus handling › should not switch focus back to the bound focus element

    expect(received).toBe(expected) // Object.is equality

    Expected: <button />
    Received: null

      77 |                      const mountedComposite = renderer.create( <Composite /> );
      78 | 
    > 79 |                      expect( getInstance( mountedComposite ).activeElementOnMount ).toBe(
         |                                                                                     ^
      80 |                              activeElement
      81 |                      );
      82 | 

      at Object.<anonymous> (packages/components/src/higher-order/with-focus-return/test/index.js:79:67)

  ● withFocusReturn() › testing rendering and focus handling › should switch focus back when unmounted while having focus

    expect(received).toBe(expected) // Object.is equality

    Expected: <button />
    Received: null

       99 |                     // Should return to the activeElement saved with this component.
      100 |                     wrapper.unmount();
    > 101 |                     expect( document.activeElement ).toBe( activeElement );
          |                                                      ^
      102 |             } );
      103 | 
      104 |             it( 'should switch focus to the most recent still-available focus target', () => {

      at Object.<anonymous> (packages/components/src/higher-order/with-focus-return/test/index.js:101:37)

  ● withFocusReturn() › testing rendering and focus handling › should switch focus to the most recent still-available focus target

    expect(jest.fn()).toHaveBeenCalled()

    Expected number of calls: >= 1
    Received number of calls:    0

      140 |                     expect(
      141 |                             wrapper.find( 'input[name="first"]' ).getDOMNode().focus
    > 142 |                     ).toHaveBeenCalled();
          |                       ^
      143 |             } );
      144 |     } );
      145 | } );

      at Object.<anonymous> (packages/components/src/higher-order/with-focus-return/test/index.js:142:6)

 FAIL  packages/components/src/popover/test/index.js
  ● Popover › should focus when opening in response to keyboard event

    expect(received).toBe(expected) // Object.is equality

    Expected: <div class="components-popover__content" tabindex="-1" />
    Received: null

      48 |                              'components-popover__content'
      49 |                      );
    > 50 |                      expect( document.activeElement ).toBe( content );
         |                                                       ^
      51 |              } );
      52 |      } );
      53 | 

      at packages/components/src/popover/test/index.js:50:37
      at batchedUpdates$1 (node_modules/react-dom/cjs/react-dom.development.js:21643:12)
      at act (node_modules/react-dom/cjs/react-dom-test-utils.development.js:1002:14)
      at Object.act (node_modules/react-dom/cjs/react-dom-test-utils.development.js:1418:12)
      at Object.<anonymous> (packages/components/src/popover/test/index.js:41:13)

 FAIL  packages/components/src/toolbar/test/index.js
  ● Toolbar › ToolbarGroup › should render an empty node, when controls are not passed

    expect(received).toBeNull()

    Received: ""

      29 |              it( 'should render an empty node, when controls are not passed', () => {
      30 |                      const wrapper = mount( <Toolbar /> );
    > 31 |                      expect( wrapper.html() ).toBeNull();
         |                                               ^
      32 |              } );
      33 | 
      34 |              it( 'should render an empty node, when controls are empty', () => {

      at Object.<anonymous> (packages/components/src/toolbar/test/index.js:31:29)

  ● Toolbar › ToolbarGroup › should render an empty node, when controls are empty

    expect(received).toBeNull()

    Received: ""

      34 |              it( 'should render an empty node, when controls are empty', () => {
      35 |                      const wrapper = mount( <Toolbar controls={ [] } /> );
    > 36 |                      expect( wrapper.html() ).toBeNull();
         |                                               ^
      37 |              } );
      38 | 
      39 |              it( 'should render a list of controls with buttons', () => {

      at Object.<anonymous> (packages/components/src/toolbar/test/index.js:36:29)


Test Suites: 6 failed, 349 passed, 355 total
Tests:       14 failed, 3752 passed, 3766 total
Snapshots:   287 passed, 287 total
Time:        23.799s, estimated 100s

@github-actions
Copy link

github-actions bot commented Mar 10, 2020

Size Change: 0 B

Total Size: 890 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.8 kB 0 B
build/autop/index.js 2.59 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 103 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.23 kB 0 B
build/block-library/editor.css 7.23 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.42 kB 0 B
build/block-library/style.css 7.43 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 196 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.24 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 2.75 kB 0 B
build/edit-navigation/style-rtl.css 279 B 0 B
build/edit-navigation/style.css 280 B 0 B
build/edit-post/index.js 92.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.1 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.18 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@diegohaz
Copy link
Member

I would try to install react-testing-library and convert one of those tests. If the test passes, that's a good signal to merge one of the other react-testing-library PRs.

@gziolo
Copy link
Member Author

gziolo commented Mar 30, 2020

I would try to install react-testing-library and convert one of those tests. If the test passes, that's a good signal to merge one of the other react-testing-library PRs.

You read my mind. I had the same thought this weekend after reading that Enzyme is going to be maintained by the community. I’d be in favor of landing your PR that introduces RTL and then we can refactor those tests as follow-up. It should unblock this PR.

@gziolo gziolo force-pushed the update/jest-major-upgrade branch from 329f170 to de8f51a Compare April 6, 2020 13:50
@gziolo
Copy link
Member Author

gziolo commented Apr 6, 2020

@getdave and @diegohaz – it looks like failing tests start to work when ported to React Testing Library.

Feel free to help fix failing tests by moving them to React Testing Library, I'm finishing work for today :)

@gziolo gziolo changed the title Chore: Major version upgrade for Jest in all packages Jest Preset: Major version upgrade for Jest in all packages Apr 7, 2020
@gziolo gziolo added the [Tool] Jest preset /packages/jest-preset-default label Apr 7, 2020
@gziolo gziolo force-pushed the update/jest-major-upgrade branch 2 times, most recently from fe59e79 to 2b683cf Compare April 8, 2020 08:07
@gziolo
Copy link
Member Author

gziolo commented Apr 8, 2020

There is only one failure left:

 FAIL  packages/components/src/sandbox/test/index.js
  ● should rerender with new emdeded content if html prop changes

    expect(jest.fn()).not.toHaveErrored(expected)

    Expected mock function not to be called but it was called with:
    ["Error: Uncaught [TypeError: Cannot read property 'body' of undefined]
        at reportException (/Users/gziolo/Projects/gutenberg/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:62:24)
        at notifyMutationObservers (/Users/gziolo/Projects/gutenberg/node_modules/jsdom/lib/jsdom/living/helpers/mutation-observers.js:166:11)
        at /Users/gziolo/Projects/gutenberg/node_modules/jsdom/lib/jsdom/living/helpers/mutation-observers.js:133:5
        at processTicksAndRejections (internal/process/task_queues.js:97:5)", [TypeError: Cannot read property 'body' of undefined]]

      34 |      function assertExpectedCalls() {
      35 |              if ( spy.assertionsNumber === 0 && spy.mock.calls.length > 0 ) {
    > 36 |                      expect( console ).not[ matcherName ]();
         |                      ^
      37 |              }
      38 |      }
      39 | 

      at Object.assertExpectedCalls (packages/jest-console/src/index.js:36:4)


Test Suites: 1 failed, 374 passed, 375 total
Tests:       1 failed, 4141 passed, 4142 total
Snapshots:   286 passed, 286 total
Time:        34.226s

@gziolo gziolo force-pushed the update/jest-major-upgrade branch from 2b683cf to 3a9ec79 Compare April 8, 2020 13:41
packages/components/src/navigable-container/test/menu.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
packages/components/src/sandbox/test/index.js Outdated Show resolved Hide resolved
@gziolo gziolo force-pushed the update/jest-major-upgrade branch from 3a9ec79 to c37eda9 Compare April 8, 2020 14:56
@gziolo
Copy link
Member Author

gziolo commented Apr 8, 2020

I sorted out all the failing tests. This PR is ready to go on the technical level. Now the question is whether we like the tests refactored to use React Testing Library? I'm happy to extract refactorings to their own PR if that help to move forward 😄

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

It's a fair point that the test refactoring could have been reserved for a separate pull request. The changes are pretty straight-forward to follow, though. I'm fine to merge as-is 👍

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
@gziolo gziolo force-pushed the update/jest-major-upgrade branch from f05ad58 to 2dc7830 Compare April 9, 2020 04:53
@gziolo gziolo merged commit 5eaa211 into master Apr 9, 2020
@gziolo gziolo deleted the update/jest-major-upgrade branch April 9, 2020 05:23
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 9, 2020
torounit added a commit that referenced this pull request Aug 18, 2022
ciampo added a commit that referenced this pull request Aug 23, 2022
* refactor storybook

* convert to typescript

* fix ComponentProps

* add changelog.

* fix imports order

* use WordPressComponentProps.

* add example

* Update packages/components/src/disabled/types.ts

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>

* Update packages/components/src/disabled/index.tsx

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>

* Update packages/components/src/disabled/index.tsx

Co-authored-by: Petter Walbø Johnsgård <petter@dekode.no>

* Update packages/components/src/disabled/index.tsx

Co-authored-by: Petter Walbø Johnsgård <petter@dekode.no>

* Remove unused imports.

* Rewrite to Testing-library `will disable all fields`

* Rewrite to Testing-library `should cleanly un-disable via reconciliation`

* use rerender

* refactor test.

* replace react-dom/test-utils to testing-library.

* remove unnecessary MutationObserver stab.
@see #20766 #20514

* Convert to typescript.

* add story for contentEditable

* add control settings.

* fix changelog

* Update packages/components/CHANGELOG.md

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>

* Omit ref.

* avoid querying

* rename div.

* test before rerender

* Simplify

* Update packages/components/src/disabled/test/index.tsx

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>

* test for sneaky DOM manipulation

* Fix `isDisabled` so that it keeps its value even if it is changed.

* add default args

* Revert "Fix `isDisabled` so that it keeps its value even if it is changed."

This reverts commit 28820f0.

* Update packages/components/src/disabled/test/index.tsx

* Update packages/components/CHANGELOG.md

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
Co-authored-by: Petter Walbø Johnsgård <petter@dekode.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] Jest preset /packages/jest-preset-default [Tool] WP Scripts /packages/scripts [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.

3 participants