Skip to content

chore: Migrate tests to RTL (4) #7044

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
Merged
nicolethoen merged 3 commits into from
Apr 8, 2022
Merged

chore: Migrate tests to RTL (4) #7044

nicolethoen merged 3 commits into from
Apr 8, 2022

Conversation

@jpuzz0
Copy link
Contributor

@jpuzz0 jpuzz0 commented Mar 10, 2022

What: Closes #7042

@jpuzz0 jpuzz0 force-pushed the chore/rtl-migrate-bundle-4 branch from 4868aef to d0e6a12 Compare March 10, 2022 17:30
@patternfly-build
Copy link
Collaborator

patternfly-build commented Mar 10, 2022 •
edited
Loading

Preview: https://patternfly-react-pr-7044.surge.sh

A11y report: https://patternfly-react-pr-7044-a11y.surge.sh

@wise-king-sullyman wise-king-sullyman self-requested a review March 23, 2022 21:01
@thatblindgeye thatblindgeye self-requested a review March 28, 2022 15:50
kmcfaul
kmcfaul reviewed Mar 28, 2022
View reviewed changes
packages/react-core/src/components/List/__tests__/List.test.tsx
expect(view.find('ol').length).toBe(1);
});

test('ordered list starts with 2nd item', () => {
Copy link
Contributor

@kmcfaul kmcfaul Mar 28, 2022

Choose a reason for hiding this comment

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

Should these 2 tests still be included to test start and type properties?

Copy link
Contributor Author

@jpuzz0 jpuzz0 Mar 28, 2022

Choose a reason for hiding this comment

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

Yes, they should. I'll update. I was maybe thinking those weren't props that translated to ones we'd see in the DOM.

packages/react-core/src/components/List/__tests__/__snapshots__/List.test.tsx.snap Outdated
</ul>
</div>
`;
exports[`List icon large list 1`] = `"<ul data-testid=\\"list-test-id\\" class=\\"pf-c-list pf-m-icon-lg\\"><li class=\\"pf-c-list__item\\"><span class=\\"pf-c-list__item-icon\\"><svg style=\\"vertical-align: -0.125em;\\" fill=\\"currentColor\\" height=\\"1em\\" width=\\"1em\\" viewBox=\\"0 0 576 512\\" aria-hidden=\\"true\\" role=\\"img\\"><path d=\\"M542.22 32.05c-54.8 3.11-163.72 14.43-230.96 55.59-4.64 2.84-7.27 7.89-7.27 13.17v363.87c0 11.55 12.63 18.85 23.28 13.49 69.18-34.82 169.23-44.32 218.7-46.92 16.89-.89 30.02-14.43 30.02-30.66V62.75c.01-17.71-15.35-31.74-33.77-30.7zM264.73 87.64C197.5 46.48 88.58 35.17 33.78 32.05 15.36 31.01 0 45.04 0 62.75V400.6c0 16.24 13.13 29.78 30.02 30.66 49.49 2.6 149.59 12.11 218.77 46.95 10.62 5.35 23.21-1.94 23.21-13.46V100.63c0-5.29-2.62-10.14-7.27-12.99z\\"></path></svg></span>Apple</li><li class=\\"pf-c-list__item\\"><span class=\\"pf-c-list__item-icon\\"><svg style=\\"vertical-align: -0.125em;\\" fill=\\"currentColor\\" height=\\"1em\\" width=\\"1em\\" viewBox=\\"0 0 1024 1024\\" aria-hidden=\\"true\\" role=\\"img\\"><path d=\\"M802,320 C748.980664,320 706,277.019336 706,224 C706,170.980664 748.980664,128 802,128 C855.019336,128 898,170.980664 898,224 C898,277.019336 855.019336,320 802,320 M704,0 C527.3,0 384,143.3 384,320 C383.937788,357.490503 390.505571,394.696657 403.4,429.9 L0,824.1 L0,1024 L192,1024 L192,896 L320,896 L320,768 L448,768 L597,622 C596.906403,621.881923 596.838304,621.745723 596.8,621.6 C631.220126,633.811107 667.47802,640.034477 704,640 C880.7,640 1024,496.7 1024,320 C1024,143.3 880.7,0 704,0\\"></path></svg></span>Banana</li><li class=\\"pf-c-list__item\\"><span class=\\"pf-c-list__item-icon\\"><svg style=\\"vertical-align: -0.125em;\\" fill=\\"currentColor\\" height=\\"1em\\" width=\\"1em\\" viewBox=\\"0 0 576 512\\" aria-hidden=\\"true\\" role=\\"img\\"><path d=\\"M528 0H48C21.5 0 0 21.5 0 48v320c0 26.5 21.5 48 48 48h192l-16 48h-72c-13.3 0-24 10.7-24 24s10.7 24 24 24h272c13.3 0 24-10.7 24-24s-10.7-24-24-24h-72l-16-48h192c26.5 0 48-21.5 48-48V48c0-26.5-21.5-48-48-48zm-16 352H64V64h448v288z\\"></path></svg></span>Orange</li></ul>"`;
Copy link
Contributor

@kmcfaul kmcfaul Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

More a general question - do these snapshots display as a single line in the IDE as well, and why? It's a bit harder to parse. Not a blocker.

Copy link
Contributor Author

@jpuzz0 jpuzz0 Mar 28, 2022

Choose a reason for hiding this comment

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

If you were to console.log the same to read in your terminal, it would be in 1 line, but without any of the escape characters.

The plan is to update all snapshots to use asFragment returned from render to make them more readable. I can maybe do that for the remaining PRs to make reviewing easier.

thatblindgeye
thatblindgeye reviewed Mar 29, 2022
View reviewed changes
Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

Some comments. If there's anything you think can be held off on until later just let me know.

...ages/react-core/src/components/NotificationDrawer/__tests__/NotificationDrawerGroup.test.tsx
test('drawer group with isRead applied ', () => {
const view = render(<NotificationDrawerGroup count={2} isExpanded={false} isRead={true} title="Critical Alerts" />);
expect(view.container).toMatchSnapshot();
test('drawer group with isRead applied ', () => {
Copy link
Contributor

@thatblindgeye thatblindgeye Mar 29, 2022

Choose a reason for hiding this comment

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

Same as above re: doesn't seem to be testing that any class is applied for being read.

@jpuzz0 jpuzz0 force-pushed the chore/rtl-migrate-bundle-4 branch from d0e6a12 to 925678d Compare March 30, 2022 16:08
@jpuzz0 jpuzz0 force-pushed the chore/rtl-migrate-bundle-4 branch from 15bd372 to fc7fce9 Compare March 30, 2022 17:29
@nicolethoen nicolethoen merged commit f40acdb into patternfly:main Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kmcfaul kmcfaul kmcfaul approved these changes

@thatblindgeye thatblindgeye thatblindgeye approved these changes

@wise-king-sullyman wise-king-sullyman Awaiting requested review from wise-king-sullyman

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Migrate tests to RTL (4)

5 participants

@jpuzz0 @patternfly-build @kmcfaul @thatblindgeye @nicolethoen