refactor(form): convert tests to use react-testing-library

[robinzigmond]

Proposed behaviour

Form component tests should use react-testing-library

Current behaviour

tests use Enzyme

Checklist

• Commits follow our style guide
• Related issues linked in commit messages if required
• Screenshots are included in the PR if useful
• All themes are supported if required
• Unit tests added or updated if required
• Playwright automation tests added or updated if required
• Storybook added or updated if required
• Translations added or updated (including creating or amending translation keys table in storybook) if required
• Typescript d.ts file added or updated if required
• Related docs have been updated if required

QA

• Tested in provided StackBlitz sandbox/Storybook
• Add new Playwright test coverage if required
• Carbon implementation matches Design System/designs
• UI Tests GitHub check reviewed if required

Additional context

Testing instructions

[nuria1110]

Looking good 👍, just a couple non-blocking comments

[nuria1110]

nit-pick (non-blocking): Since saveButton is already being tested separately I don't think its necessary to also test it here.

[robinzigmond]

So the idea here is that we're checking that the leftSideButtons are actually on the left - ie they appear before the Save button. So I think it's important to render the Save button here to check that the first 2 buttons (in DOM order) are the left buttons, and that the save button comes next. I take the point that maybe the assertion that the third button (footerButtons[2]) is the save button isn't necessary, but personally I think having that there in the assertions makes it clearer what is being tested here, ie that the left buttons come first, then the save button.

Happy to wait to see what a second reviewer thinks though.

[edleeks87]

I can see both sides here tbh, I think we have the left and right side buttons captured in chromatic so the order isn't critical to me but it's not a huge issue to test that as it's trivial etc

[nuria1110]

suggestion (non-blocking): If only the saveButton is being rendered we could just assert that the button is visible:

Suggested change

	test("renders the button passed as the saveButton prop in the form footer", () => {
	render(<Form saveButton={<Button>Custom Save Button</Button>} />);
	const footerButton = within(screen.getByTestId("form-footer")).getByRole(
	"button"
	);
	expect(footerButton).toHaveTextContent("Custom Save Button");
	});
	test("renders the button passed as the saveButton prop in the form footer", () => {
	render(<Form saveButton={<Button>Custom Save Button</Button>} />);
	expect(screen.getByRole("button", { name: "Custom Save Button" }).toBeVisible();
	});

[robinzigmond]

I think the within is important here, because part of the test is that the button is rendered inside the form footer. But you're right that using the name option and a toBeVisible check is probably better than generically finding a button and checking its text content, so I'm happy to change that 👍

[edleeks87]

Yep agree within with toBeVisible works best imo

[nuria1110]

nit-pick (non-blocking): Same as above, I don't think its necessary to also test saveButton here.

[robinzigmond]

same reply as above 😃

[edleeks87]

praise: good idea moving this to chromatic 👏

[carbonci]

🎉 This PR is included in version 142.9.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀