Flakey unit tests for CreatePipelineRun.test.js

[ncskier]

In my latest Pull Request, I saw a failure with the CreatePipelineRun tests even though I had not modified the CreatePipelineRun file. After running the unit tests again with /test tekton-dashboard-unit-tests, the tests passed. This indicates that the unit tests are flakey.

Here's a link to the Pull Request where I saw the test failure: #906 (comment)

Here's a link to the Prow logs for the unit test: https://tekton-releases.appspot.com/build/tekton-prow/pr-logs/pull/tektoncd_dashboard/906/pull-tekton-dashboard-unit-tests/1217887100729298944/

This is a snippet from the logs:

I0116 19:19:51.136] FAIL src/containers/CreatePipelineRun/CreatePipelineRun.test.js (72.894s)
I0116 19:19:51.136]   ● CreatePipelineRun renders pipeline controlled
I0116 19:19:51.136] 
I0116 19:19:51.137]     expect(received).toBeFalsy()
I0116 19:19:51.137] 
I0116 19:19:51.137]     Received: <div style="display: block;">namespace-1</div>
I0116 19:19:51.138] 
I0116 19:19:51.138]       421 |     </Provider>
I0116 19:19:51.138]       422 |   );
I0116 19:19:51.139]     > 423 |   expect(queryByText(/namespace-1/i)).toBeFalsy();
I0116 19:19:51.139]           |                                       ^
I0116 19:19:51.139]       424 |   expect(queryByText(/pipeline-1/i)).toBeTruthy();
I0116 19:19:51.140]       425 |   // Verify spec details are displayed
I0116 19:19:51.140]       426 |   testPipelineSpec('id-pipeline-1', queryByText, queryByValue);
I0116 19:19:51.141] 
I0116 19:19:51.141]       at Object.toBeFalsy (src/containers/CreatePipelineRun/CreatePipelineRun.test.js:423:39)

[AlanGreene]

I've been hit by this a few times this week so assigning myself. Can't reproduce locally but will submit some changes I suspect will help with at least one of the failures.

[AlanGreene]

Closing since #939 is merged, can reopen if we still see issues with that.

[AlanGreene]

Reopening, still seeing failures in CreatePipelineRun.test.js

https://storage.googleapis.com/tekton-prow/pr-logs/pull/tektoncd_dashboard/901/pull-tekton-dashboard-unit-tests/1220706050584350720/build-log.txt

I0124 13:58:25.014] FAIL src/containers/CreatePipelineRun/CreatePipelineRun.test.js (47.15s)
I0124 13:58:25.015]   � CreatePipelineRun › renders pipeline controlled
I0124 13:58:25.015] 
I0124 13:58:25.015]     expect(received).toBeFalsy()
I0124 13:58:25.015] 
I0124 13:58:25.016]     Received: <div style="display: block;">namespace-1</div>
I0124 13:58:25.016] 
I0124 13:58:25.019]       423 |       </Provider>
I0124 13:58:25.019]       424 |     );
I0124 13:58:25.020]     > 425 |     expect(queryByText(/namespace-1/i)).toBeFalsy();
I0124 13:58:25.020]           |                                         ^
I0124 13:58:25.020]       426 |     expect(queryByText(/pipeline-1/i)).toBeTruthy();
I0124 13:58:25.020]       427 |     // Verify spec details are displayed
I0124 13:58:25.021]       428 |     testPipelineSpec('id-pipeline-1', queryByText, queryByValue);
I0124 13:58:25.021] 
I0124 13:58:25.021]       at Object.toBeFalsy (src/containers/CreatePipelineRun/CreatePipelineRun.test.js:425:41)
I0124 13:58:25.021] 
I0124 13:58:25.022]   � CreatePipelineRun › validates inputs
I0124 13:58:25.022] 
I0124 13:58:25.022]     `initialFocus` did not return a node
I0124 13:58:25.022] 
I0124 13:58:25.023]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0124 13:58:25.023]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0124 13:58:25.023]       at node_modules/focus-trap/index.js:143:16
I0124 13:58:25.024]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0124 13:58:25.024] 

etc.

#901

[AlanGreene]

This one just won't go away... 😣
Hit it 4 or 5 times today out of ~20 builds, but never locally...

https://storage.googleapis.com/tekton-prow/pr-logs/pull/tektoncd_dashboard/1066/pull-tekton-dashboard-unit-tests/1229484850340171777/build-log.txt

I0217 19:24:15.601] Summary of all failing tests
I0217 19:24:15.601] FAIL src/containers/CreatePipelineRun/CreatePipelineRun.test.js (37.924s)
I0217 19:24:15.601]   � CreatePipelineRun › renders pipeline controlled
I0217 19:24:15.602] 
I0217 19:24:15.602]     expect(received).toBeFalsy()
I0217 19:24:15.602] 
I0217 19:24:15.602]     Received: <div style="display: block;">namespace-1</div>
I0217 19:24:15.602] 
I0217 19:24:15.603]       433 |     );
I0217 19:24:15.603]       434 |     await waitForElement(() => getByText(/pipeline-1/i));
I0217 19:24:15.603]     > 435 |     expect(queryByText(/namespace-1/i)).toBeFalsy();
I0217 19:24:15.603]           |                                         ^
I0217 19:24:15.604]       436 |     expect(queryByText(/pipeline-1/i)).toBeTruthy();
I0217 19:24:15.604]       437 |     // Verify spec details are displayed
I0217 19:24:15.604]       438 |     testPipelineSpec('id-pipeline-1', queryByText, queryByValue);
I0217 19:24:15.604] 
I0217 19:24:15.605]       at Object.it (src/containers/CreatePipelineRun/CreatePipelineRun.test.js:435:41)
I0217 19:24:15.605] 
I0217 19:24:15.605]   � CreatePipelineRun › validates inputs
I0217 19:24:15.605] 
I0217 19:24:15.606]     `initialFocus` did not return a node
I0217 19:24:15.606] 
I0217 19:24:15.606]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.606]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.607]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.607]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0217 19:24:15.607] 
I0217 19:24:15.607]   � CreatePipelineRun › validates inputs
I0217 19:24:15.608] 
I0217 19:24:15.608]     `initialFocus` did not return a node
I0217 19:24:15.608] 
I0217 19:24:15.608]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.608]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.612]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.612]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0217 19:24:15.612] 
I0217 19:24:15.613]   � CreatePipelineRun › validates inputs
I0217 19:24:15.613] 
I0217 19:24:15.613]     `initialFocus` did not return a node
I0217 19:24:15.613] 
I0217 19:24:15.614]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.614]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.614]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.615]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0217 19:24:15.615] 
I0217 19:24:15.615]   � CreatePipelineRun › validates inputs
I0217 19:24:15.616] 
I0217 19:24:15.616]     `initialFocus` did not return a node
I0217 19:24:15.616] 
I0217 19:24:15.616]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.617]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.617]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.617]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0217 19:24:15.618] 
I0217 19:24:15.618]   � CreatePipelineRun › validates inputs
I0217 19:24:15.618] 
I0217 19:24:15.619]     `initialFocus` did not return a node
I0217 19:24:15.619] 
I0217 19:24:15.619]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.620]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.620]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.620]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0217 19:24:15.621] 
I0217 19:24:15.621]   � CreatePipelineRun › validates inputs
I0217 19:24:15.621] 
I0217 19:24:15.621]     `initialFocus` did not return a node
I0217 19:24:15.622] 
I0217 19:24:15.622]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.622]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.623]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.623]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
I0217 19:24:15.623] 
I0217 19:24:15.624]   � CreatePipelineRun › validates inputs
I0217 19:24:15.624] 
I0217 19:24:15.624]     `initialFocus` did not return a node
I0217 19:24:15.624] 
I0217 19:24:15.625]       at getNodeForOption (node_modules/focus-trap/index.js:181:15)
I0217 19:24:15.625]       at getInitialFocusNode (node_modules/focus-trap/index.js:189:9)
I0217 19:24:15.625]       at node_modules/focus-trap/index.js:143:16
I0217 19:24:15.626]       at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)

@a-roberts did I see you post somewhere recently about a similar issue with tests involving the Carbon Modal?

[a-roberts]

https://storage.googleapis.com/tekton-prow/pr-logs/pull/tektoncd_dashboard/1066/pull-tekton-dashboard-unit-tests/1229484850340171777/build-log.txt

I'm seeing this locally consistently now, a colleague at IBM mentioned this in Slack in response

Jeff Arn  12 days ago
I know this is an old thread, but I wanted to share some of my findings based on running into this in @testing-library/react.

It appears that the error stack is related to this code in focus-trap
https://github.com/davidtheclark/focus-trap/blob/4.0.2/index.js#L189

This calls getNodeForOption()
https://github.com/davidtheclark/focus-trap/blob/4.0.2/index.js#L189

The issue I'm running into occurs when running tests that fireEvent s from the testing library.
When I log out the results from getNodeForOption(), I see that the initial test render appropriately returns the HTMLButtonElement for Close20 (which I believe is the default focus item).

Then, when I fireEvent.click , in this case on the Close20 button, I notice getNodeForOption() gets called again, but this time it throws the error originally posted, coming from this line:
https://github.com/davidtheclark/focus-trap/blob/4.0.2/index.js#L181

In this case, I believe optionValue corresponds to initialFocus() from Carbon React's Modal component:
https://github.com/carbon-design-system/carbon/blob/master/packages/react/src/components/Modal/Modal.js#L247-L258

So, I think there's some weird behavior happening when the component gets rendered in test environments, where the mount lifecycle method gets called again after events where the modal has been closed, making all the refs null.

Interestingly enough, I don't think that is actually bad behavior, I think that focus-trap is actually throwing an error that is bubbling up into the test runner, failing the test.

Commenting out this line https://github.com/davidtheclark/focus-trap/blob/4.0.2/index.js#L181 causes the test to pass while assertions confirm that it is behaving appropriately

I'm still investigating, but I think the solution might be to Jest mock out that method in the focus-trap package, and have it not throw errors and instead return null or something similar

I'm writing a mock now, if it works I'll share it

Jeff Arn  12 days ago
jest.mock('focus-trap', () => (() => ({
  activate: jest.fn(),
  deactivate: jest.fn(),
  pause: jest.fn(),
  unpause: jest.fn(),
  addListeners: jest.fn(),
  removeListeners: jest.fn(),
  getNodeForOption: jest.fn(),
  getInitialFocusNode: jest.fn(),
  checkPointerDown: jest.fn(),
  checkFocusIn: jest.fn(),
  checkKey: jest.fn(),
  checkTab: jest.fn(),
  checkClick: jest.fn(),
  updateTabbableNodes: jest.fn(),
  tryFocus: jest.fn(),
})));

This pretty much just mocks out the focus-trap interface. It's a pretty brute force solution since it removes all functionality of focus-trap, so if you rely on asserting focus trap behavior in your tests this won't work for you.

Some of the jest.fn()'s could be extended with a .mockImplementation() if you needed functionality from them.

But my original tests that relied on this behavior are now passing:
    test('Autoprovision Modal closes', () => {
      const initialState = {
        modals: {
          modalStack: [
            {
              modalType: 'AUTOPROVISION',
              modalProps: null,
            },
          ],
        },
      };
      const { getByTitle, store } = renderWithRedux(
        <ModalConductor t={noop} />,
        { initialState },
      );
      fireEvent.click(getByTitle('close the modal'));
      expect(store.getState().modals.modalStack).toHaveLength(0);
    });

[AlanGreene]

PR merged to mock focus-trap, and just as that went in I spotted this Carbon PR that actually removes focus-trap-react: carbon-design-system/carbon#5260

So once the next Carbon release goes out in a week or two we should be able to remove this mock.