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

[Task] Revamp unit test framework to isolate snapshot testing #661

Open
Tracked by #848
tthvo opened this issue Nov 16, 2022 · 2 comments
Open
Tracked by #848

[Task] Revamp unit test framework to isolate snapshot testing #661

tthvo opened this issue Nov 16, 2022 · 2 comments
Labels
help wanted Extra attention is needed test

Comments

@tthvo
Copy link
Member

tthvo commented Nov 16, 2022

Related to #648

It appears sometimes that including snapshot testing in the same test suite with other tests causes some irregularities in test setup (i.e. setState on unmounted component, or TypeError: unable to read property from null (reading children).

To reproduce a simple example:

  1. Go to src/test/Recordings/Recordings.test.tsx.
  2. Move the snapshot test up front as the first test.
  3. Add .mockReturnValueOnce(of(true)) as first call to .spyOn(defaultServices.api, 'isArchiveEnabled') for mocking this service call.
  4. Run the test suite
    $ yarn test -t "<Recordings /> -u " // Need to update snapshot
  5. See error:
TypeError: Cannot read properties of null (reading 'children')

      at Tabs._this.countOverflowingElements (node_modules/@patternfly/react-core/dist/js/components/Tabs/Tabs.js:35:51)
      at Tabs.Object.<anonymous>.Tabs.componentDidUpdate (node_modules/@patternfly/react-core/dist/js/components/Tabs/Tabs.js:191:49)
      at commitLifeCycles (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:10449:22)
      at commitLayoutEffects (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:13295:7)
      at HTMLUnknownElement.callCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9972:14)
      at HTMLUnknownElement.callTheUserObjectsOperation (node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30)
      at innerInvokeEventListeners (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:338:25)
      at invokeEventListeners (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:274:3)
      at HTMLUnknownElementImpl._dispatch (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:221:9)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:94:17)
      at HTMLUnknownElement.dispatchEvent (node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:231:34)
      at Object.invokeGuardedCallbackDev (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:10021:16)
      at invokeGuardedCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:10073:31)
      at commitRootImpl (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:13033:9)
      at unstable_runWithPriority (node_modules/react-test-renderer/node_modules/scheduler/cjs/scheduler.development.js:653:12)
      at runWithPriority (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1775:10)
      at commitRoot (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:12873:3)
      at finishSyncRender (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:12384:3)
      at performSyncWorkOnRoot (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:12370:7)
      at node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1825:24
      at unstable_runWithPriority (node_modules/react-test-renderer/node_modules/scheduler/cjs/scheduler.development.js:653:12)
      at runWithPriority (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1775:10)
      at flushSyncCallbackQueueImpl (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1820:7)
      at flushSyncCallbackQueue (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1808:3)
      at scheduleUpdateOnFiber (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:11776:9)
      at Object.enqueueSetState (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:3401:5)
      at Tabs.Object.<anonymous>.Component.setState (node_modules/react/cjs/react.development.js:365:16)
      at node_modules/@patternfly/react-core/dist/js/components/Tabs/Tabs.js:60:22
      at Timeout.task [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:516:19)

Though this could be due to how we mock modules. Need investigating.

@tthvo
Copy link
Member Author

tthvo commented Dec 13, 2022

Failed run: https://github.com/cryostatio/cryostat-web/actions/runs/3660946309/jobs/6188646168

FAIL src/test/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.test.tsx
  ● <ClickableAutomatedAnalysisLabel /> › displays popover when critical label is clicked

    TestingLibraryElementError: Unable to find an accessible element with the role "button" and name `/close/i`

    There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the `hidden` option to `true`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

    Ignored nodes: comments, script, style
    <body>
      <div>
        <span
          aria-label="rule1"
          class="pf-c-label pf-m-red pf-m-compact"
          style="cursor: pointer; --pf-c-label__content--before--BorderWidth: 2.5px; --pf-c-label__content--before--BorderColor: #06c;"
        >
          <span
            class="pf-c-label__content"
          >
            <span
              class="pf-c-label__icon"
            >
              <svg
                aria-hidden="true"
                fill="currentColor"
                height="1em"
                role="img"
                style="vertical-align: -0.125em;"
                viewBox="0 0 512 512"
                width="1em"
              >
                <path
                  d="M256 8C119.043 8 8 119.083 8 256c0 136.997 111.043 248 248 248s248-111.003 248-248C504 119.083 392.957 8 256 8zm0 110c23.196 0 42 18.804 42 42s-18.804 42-42 42-42-18.804-42-42 18.804-42 42-42zm56 254c0 6.627-5.373 12-12 12h-88c-6.627 0-12-5.373-12-12v-24c0-6.627 5.373-12 12-12h12v-64h-12c-6.627 0-12-5.373-12-12v-24c0-6.627 5.373-12 12-12h64c6.627 0 12 5.373 12 12v100h12c6.627 0 12 5.373 12 12v24z"
                />
              </svg>
            </span>
            <span
              class="clickable-automated-analysis-label-name"
            >
              rule1
            </span>
          </span>
        </span>
      </div>
    </body>

       99 |     await user.click(screen.getByText(mockRuleEvaluation1.name));
      100 |
    > 101 |     const closeButton = screen.getByRole('button', {
          |                                ^
      102 |       name: /close/i,
      103 |     });
      104 |

      at Object.getElementError (node_modules/@testing-library/dom/dist/config.js:40:19)
      at node_modules/@testing-library/dom/dist/query-helpers.js:90:38
      at node_modules/@testing-library/dom/dist/query-helpers.js:62:17
      at node_modules/@testing-library/dom/dist/query-helpers.js:111:19
      at src/test/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.test.tsx:101:32
      at step (src/test/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.test.tsx:70:23)
      at Object.next (src/test/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.test.tsx:51:53)
      at fulfilled (src/test/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.test.tsx:42:58)

There has been some flanky runs with this tests locally and in github Actions. Need to take a closer look.

@tthvo
Copy link
Member Author

tthvo commented Mar 21, 2023

$ yarn test -t '<NotificationControl/>' --verbose
  console.warn
    [mobx-react-lite] importing batchingForReactDom is no longer needed

      at Object.<anonymous> (node_modules/mobx-react-lite/batchingForReactDom.js:2:13)
      at Object.<anonymous> (node_modules/mobx-react/batchingForReactDom.js:1:83)

  console.warn
    [mobx-react-lite] importing batchingForReactDom is no longer needed

      at Object.<anonymous> (node_modules/mobx-react-lite/batchingForReactDom.js:2:13)
      at Object.<anonymous> (node_modules/mobx-react/batchingForReactDom.js:1:83)

  console.error
    Warning: It looks like you're using the wrong act() around your test interactions.
    Be sure to use the matching version of act() corresponding to your renderer:
    
    // for react-dom:
    import {act} from 'react-dom/test-utils';
    // ...
    act(() => ...);
    
    // for react-test-renderer:
    import TestRenderer from 'react-test-renderer';
    const {act} = TestRenderer;
    // ...
    act(() => ...);
        in Component

      at console.error (node_modules/@testing-library/react/dist/act-compat.js:55:34)
      at printWarning (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:120:30)
      at error (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:92:5)
      at warnIfNotScopedWithMatchingAct (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:13714:7)
      at dispatchAction (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6404:9)
      at ConsumerObserver.Object.<anonymous>.ConsumerObserver.next (node_modules/rxjs/src/internal/Subscriber.ts:161:25)
      at SafeSubscriber.Object.<anonymous>.Subscriber._next (node_modules/rxjs/src/internal/

@andrewazores andrewazores moved this from Stretch Goals to Pushed to 2.4.0 in 2.3.0 release Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed test
Projects
No open projects
Status: Pushed to 2.4.0
Development

No branches or pull requests

1 participant