Lattice-Automation/seqviz

Fix `testing-library/react` weirdness: `act(...)` and async render

mattwongasimov opened this issue · 12 comments

Describe the bug

When I try to test SeqViz in testing-library/react the rendered output of my component:

<SeqViz
      annotations={annotations}
      seq={sequence}
      viewer="linear"
    />

is :

TestingLibraryElementError: Unable to find an element with the text: ACACGCGCCTCTCACTATAATCAATGACTAAAGCGTGCCTCGTCGCAGATTAGTGAGGATCGCTCGGCTGTGAGTCCAACTAATTCCCCCACCTAAATGAGCCCGTTCGCATCGGGTATGGTGGGGGGTAGGTCCCCACCTTGTTGCTTGAGTAACCGCGATTGGGTGATCTTTTGAAGGACCGTAATCAAACGTTCCTGGATCCAGAGGAGGGATGTTTGGCCCGAGATAGTACGCCGGTGGCATCTCGAGCTGGGGGTGCGGCTCAATGAAATCTGAGGTGGAGAGAGAGGGATTGTATAAGAAAAGTGCCAGTTCTTCCCCGCAGGAATATATGGTTAGGGCTTTGCACAAACTCGACCAATTTCTTTTCTCTACTGCTCTCGGTACGACTGGGTATCGTACACTCTTTGCTTTCCTTTTATTA. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.
    <body>
      <div>
        <div
          class="la-vz-seqviz"
        />
      </div>
    </body>

Expected behavior

I would expect the rendered content to be what the Chrome browser renders:

image

Screenshots

Your environment:

My test file:

// A testing library react test for SequenceViewer.tsx component
import { cleanup, render, screen } from '@testing-library/react';

import SequenceViewer from '@components/SequenceViewer';

const sequence =
  'ACACGCGCCTCTCACTATAATCAATGACTAAAGCGTGCCTCGTCGCAGATTAGTGAGGATCGCTCGGCTGTGAGTCCAACTAATTCCCCCACCTAAATGAGCCCGTTCGCATCGGGTATGGTGGGGGGTAGGTCCCCACCTTGTTGCTTGAGTAACCGCGATTGGGTGATCTTTTGAAGGACCGTAATCAAACGTTCCTGGATCCAGAGGAGGGATGTTTGGCCCGAGATAGTACGCCGGTGGCATCTCGAGCTGGGGGTGCGGCTCAATGAAATCTGAGGTGGAGAGAGAGGGATTGTATAAGAAAAGTGCCAGTTCTTCCCCGCAGGAATATATGGTTAGGGCTTTGCACAAACTCGACCAATTTCTTTTCTCTACTGCTCTCGGTACGACTGGGTATCGTACACTCTTTGCTTTCCTTTTATTA';

const item = {
  sequence,
};

describe('SequenceViewer', () => {
  afterEach(() => {
    cleanup();
  });

  it('should display the correct testIds', async () => {
    render(<SequenceViewer payload={item} />);
    expect(screen.getByText(sequence)).toBeInTheDocument();
  });
});

SequenceViewer.tsx:

import React from 'react';
import { SeqViz, SeqVizProps } from 'seqviz';

type SequenceViewerProps = {
  payload: any;
};

const SequenceViewer: React.FunctionComponent<SequenceViewerProps> = (
  props
) => {
  let sequence: string = props.payload.sequence;


  return (
    <SeqViz
      seq={sequence}
      viewer="linear"
    />
  );
};

export default SequenceViewer;

Other Errors:

console.error
      Warning: An update to SeqViz inside a test was not wrapped in act(...).
      
      When testing, code that causes React state updates should be wrapped into act(...):
      
      act(() => {
        /* fire events that update state */
      });
      /* assert on the output */
      
      This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act

I don't know why this error is occurring because I'm not doing any state updates or user interactions in my component.

jjti commented

hey @mattwongasimov, I've run into similar issues. IIUC, it's because SeqViz parses props -> state via an async call. This causes the actual viewer to render to DOM on the second React render, which trips up the testing-library.

there are examples in this file of how I test the viewer using react testing-library (with an async/await on a test id): https://github.com/Lattice-Automation/seqviz/blob/develop/src/index.test.tsx#L57-L71

When I get some time I'll try to dive deeper and root-cause, I agree this is sort of annoying

Thanks for the quick response @jjtimmons . I tried your test, but wasn't able to get it to pass. I wasn't able to get your test file to work at all, maybe it's a jest/react version issue. I'll keep an eye out for any progress notifications on this.

jjti commented

@mattwongasimov that's a good lead: which version of React, Jest/, @testing-library/react, & seqviz are you using? I can try to reproduce on my end -- assuming the diff is in one of the non-seqviz packages; if the diff is in seqviz I'd recommend bumping that first.

"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^13.4.0",
"@testing-library/user-event": "^14.4.3",
"react": "^18.2.0",
"ts-jest": "^29.0.3",
"seqviz": "^3.7.2",
jjti commented

@mattwongasimov I'm not 100% certain that it's fixed but I tried to fix it in this release: https://github.com/Lattice-Automation/seqviz/releases/tag/3.7.9

The change is to parse sequences and files synchronously (in the constructor), rather that in a second render (componentDidMount), so there shouldn't be any warning about act, etc. Again, you can see all the testing-library tests that I'm doing in here: https://github.com/Lattice-Automation/seqviz/blob/develop/src/index.test.tsx

Please let me know if this fixes the issue you'd been running into

jjti commented

This is the main reference @mattwongasimov, have to grab elements by their test ID and concatenate their sequences:

const props = {
  name: "test_part",
  seq: "ATGGTAGTTAGATAGGGATACCGATAGACTTGAGAGATACCGCATCTATTTACGACCAGCGAGCAG",
  testSize: { height: 600, width: 800 },
};

describe("SeqViz rendering (React)", () => {
  afterEach(() => cleanup());

  it("renders", () => {
    const { getAllByTestId } = render(<SeqViz {...props} />);
    expect(getAllByTestId("la-vz-seqviz")).toBeTruthy();
    expect(getAllByTestId("la-vz-viewer-container")).toHaveLength(1);

    // renders full sequence
    const seqs = getAllByTestId("la-vz-seq");
    const seq = seqs.map(s => s.textContent).join("");
    expect(seq).toEqual(props.seq);
  });

testSize is also necessary to override the default, parent element-based size detection

I was not able to fix my test after upgrading to 3.7.10.
My output of render is empty as shown:

const getAllByTestId: (id: Matcher, options?: MatcherOptions | undefined) => HTMLElement[]
SeqViz rendering (React) > renders
-----
TestingLibraryElementError: Unable to find an element by: [data-testid="la-vz-seqviz"]

Ignored nodes: comments, <script />, <style />
<body>
  <div />
</body>Jest

This is the test:

import { cleanup, render } from '@testing-library/react';
import SeqViz from 'seqviz';

const props = {
  name: 'test_part',
  seq: 'ATGGTAGTTAGATAGGGATACCGATAGACTTGAGAGATACCGCATCTATTTACGACCAGCGAGCAG',
  testSize: { height: 600, width: 800 },
};

describe('SeqViz rendering (React)', () => {
  afterEach(() => cleanup());

  it('renders', () => {
    const { getAllByTestId } = render(<SeqViz {...props} />);
    expect(getAllByTestId('la-vz-seqviz')).toBeTruthy();
    expect(getAllByTestId('la-vz-viewer-container')).toHaveLength(1);

    // renders full sequence
    const seqs = getAllByTestId('la-vz-seq');
    const seq = seqs.map((s) => s.textContent).join('');
    expect(seq).toEqual(props.seq);
  });
});

@mattwongasimov @jjti - Is this still an issue or can we close this out?

I haven't tried to test it, but once I find some free time to check this test I will update the package and try again and report back.

Hi @mattwongasimov, were you able to test this out?

Hi @mattwongasimov, were you able to test this out?

Sorry I haven't had the time but seqviz has been stable so far!

Awesome. I'll close the issue for now then. Feel free to re-open if you find this is still an issue.