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.

Sign up for GitHub

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

Jump to bottom

Avoid capturing RTL internals in DOM snapshots #157

Open
kurtdoherty opened this issue Aug 21, 2024 · 0 comments
Open

Avoid capturing RTL internals in DOM snapshots #157

kurtdoherty opened this issue Aug 21, 2024 · 0 comments
Assignees
@kurtdoherty
Labels
housekeeping The issue relates to a housekeeping task or chore

Comments

@kurtdoherty
Copy link
Contributor

kurtdoherty commented Aug 21, 2024 •
edited
Loading

Context

Currently, many of the component unit tests in Elements take a snapshot testing approach; more specifically, they take a "DOM snapshot" which capture what we expect to see in the DOM when the component is used.

For example, the Avatar component implements snapshots in the following manner:

it('should match a snapshot', () => {
  const wrapper = render(<Avatar>Some Content</Avatar>)
  expect(wrapper).toMatchSnapshot()
})

Since wrapper is the full result object from React Testing Library's (RTL) render function, we are capturing its API instead of just the DOM content:

exports[`Avatar component should match a snapshot 1`] = `
{
  "asFragment": [Function],
  "baseElement": <body>
    <div>
      <mock-styled.div
        classname=""
        role="img"
        title="A profile image"
      >
        Some Content
      </mock-styled.div>
    </div>
  </body>,
  "container": <div>
    <mock-styled.div
      classname=""
      role="img"
      title="A profile image"
    >
      Some Content
    </mock-styled.div>
  </div>,
  "debug": [Function],
  "findAllByAltText": [Function],
  "findAllByDisplayValue": [Function],
  "findAllByLabelText": [Function],
  "findAllByPlaceholderText": [Function],
  "findAllByRole": [Function],
  "findAllByTestId": [Function],
  "findAllByText": [Function],
  "findAllByTitle": [Function],
  "findByAltText": [Function],
  "findByDisplayValue": [Function],
  "findByLabelText": [Function],
  "findByPlaceholderText": [Function],
  "findByRole": [Function],
  "findByTestId": [Function],
  "findByText": [Function],
  "findByTitle": [Function],
  "getAllByAltText": [Function],
  "getAllByDisplayValue": [Function],
  "getAllByLabelText": [Function],
  "getAllByPlaceholderText": [Function],
  "getAllByRole": [Function],
  "getAllByTestId": [Function],
  "getAllByText": [Function],
  "getAllByTitle": [Function],
  "getByAltText": [Function],
  "getByDisplayValue": [Function],
  "getByLabelText": [Function],
  "getByPlaceholderText": [Function],
  "getByRole": [Function],
  "getByTestId": [Function],
  "getByText": [Function],
  "getByTitle": [Function],
  "queryAllByAltText": [Function],
  "queryAllByDisplayValue": [Function],
  "queryAllByLabelText": [Function],
  "queryAllByPlaceholderText": [Function],
  "queryAllByRole": [Function],
  "queryAllByTestId": [Function],
  "queryAllByText": [Function],
  "queryAllByTitle": [Function],
  "queryByAltText": [Function],
  "queryByDisplayValue": [Function],
  "queryByLabelText": [Function],
  "queryByPlaceholderText": [Function],
  "queryByRole": [Function],
  "queryByTestId": [Function],
  "queryByText": [Function],
  "queryByTitle": [Function],
  "rerender": [Function],
  "unmount": [Function],
}
`;

To fix this, we should avoid taking a snapshot of the whole render result object from RTL, and instead only capture the actual document fragment, which is available via the asFragment function provided in the render result. This approach was recently taken for the TextArea component's snapshots in #150 and #151.

describe('content field-sizing', () => {
  test('default min/max rows are 3 -> Infinity', () => {
    const { asFragment } = render(<TextArea fieldSizing="content" />)
    expect(asFragment()).toMatchSnapshot()
  })
})

This results in the following snapshot content:

exports[`content field-sizing default min/max rows are 3 -> Infinity 1`] = `
<DocumentFragment>
  <mock-styled.textarea
    classname=""
    data-field-sizing="content"
    style="--textarea-max-rows: Infinity; --textarea-min-rows: 3;"
  />
</DocumentFragment>
`;

Why not just snapshot wrapper.baseElement or wrapper.container?

In both cases, these include DOM elements that are implementation details of RTL and how it renders components. Further, seeing extra DOM elements in the snapshot may also confuse readers, making them incorrectly think those elements are rendered by the component under test.

In contrast, using asFragment() means we only see an extra <DocumentFragment> element in the snapshot, which is easier to recognise as being unrelated to our component under test. DocumentFragment is also part of the DOM API, so its presence in the snapshot does not represent an RTL implementation detail.

Work to be done

Given the number of components that will be impacted by this change, I'm proposing we do this as a big-bang change:

  1. Find expect(wrapper).toMatchSnapshot() and replace with expect(wrapper.asFragment()).toMatchSnapshot();
  2. Run tests and update all snapshots;
  3. PR and merge.

Thoughts?
@kurtdoherty kurtdoherty added the housekeeping The issue relates to a housekeeping task or chore label Aug 21, 2024
@kurtdoherty kurtdoherty changed the title Avoid capturing RTL internals in DOM snapshots [Unit tests] Avoid capturing RTL internals in DOM snapshots Aug 21, 2024
@kurtdoherty kurtdoherty changed the title [Unit tests] Avoid capturing RTL internals in DOM snapshots Avoid capturing RTL internals in DOM snapshots Aug 21, 2024
@kurtdoherty kurtdoherty mentioned this issue Aug 21, 2024
Open
@kurtdoherty kurtdoherty self-assigned this Aug 21, 2024
@kurtdoherty kurtdoherty mentioned this issue Oct 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kurtdoherty kurtdoherty

Labels
housekeeping The issue relates to a housekeeping task or chore
Projects
Elements Roadmap
Status: To Review
Milestone
No milestone
Development

No branches or pull requests
1 participant
@kurtdoherty