Shift mocked store enzyme test to React Testing Library #3909

nishitmistry · 2026-01-17T17:23:14Z

Starting the migration of enzyme test to @testing-library/react under task Move remaining enzyme tests to @testing-library/react #3767
Added @testing-library/jest-dom package for some handy assertion methods
Changed the mountComponentWithStore to use @testing-library/react package's render method.
Fixed the tests which broke with the change made to mountComponentWithStore method.

mathjazz · 2026-01-20T13:40:49Z

Thanks for your contribution, @nishitmistry! Is this ready for a review or do you still plan to make changes?

nishitmistry · 2026-01-21T04:57:11Z

Hey @mathjazz, I am done with the migrations of store related tests, you can go ahead with the review.

mathjazz

Nice work!

Added some high-level questions and comments. Please also rebase against main.

Thanks for your contributions to Pontoon!

mathjazz · 2026-01-23T12:25:19Z

translate/src/modules/editor/components/FailedChecks.test.jsx


 import { FailedChecks } from './FailedChecks';
 import { vi } from 'vitest';
+import { expect } from 'vitest';


Why not put everything in one line?

import { vi, expect } from 'vitest';

mathjazz · 2026-01-23T12:25:34Z

translate/src/modules/comments/components/CommentsList.test.js

    });

-    expect(wrapper.find('ul > *')).toHaveLength(3);
+    expect(container.querySelectorAll('ul > *')).toHaveLength(3);


Could you please help me understand what's the motivation for this change (and many other similar changes below)? Is find() coming from enzyme?

yes, find() is returned by the mount() method of enzyme, which locates a node based on any selector. There's no such method in React Testing Library to select a node with html selector, so we use the container's querySelector() to select the nodes.

Using querySelector() is a bit of a code smell with testing-library, tbh. It's ok to do so initially here, but it would be good to leave // TODO code comments in the vicinity of each such test to replace them with getBy* and similar testing-library-ish approaches.

mathjazz · 2026-01-23T12:26:46Z

translate/src/modules/entitieslist/components/Entity.tsx


  return (
-    <li className={cn} onClick={handleSelectEntity}>
+    <li className={cn} onClick={handleSelectEntity} data-testid='entity'>


Not a big fan of adding data-* attributes just for the sake of testing. Is it really necessary?

We can't query based on the component as we do with Enzyme ( wrapper.find(Entity); is not possible in React Testing Library). So if we wish to know whether a particular component exists or not, adding data-testid='entity' and then querying it would be the way afaik.
React Testing Library recommends an accessibility-based testing approach RTL queries priority guide, but the task is limited to just the migration.

The testid attributes do have their place, but as the page you link to itself documents, they should only be used when no other option works.

eemeli

Looks pretty decent. A few too many test ids and on occasion a bit too verbose, but decent. More comments inline.

eemeli · 2026-01-29T13:04:33Z

translate/src/modules/editor/components/FailedChecks.test.jsx

@@ -30,7 +30,9 @@ describe('<FailedChecks>', () => {
  it('does not render if no errors or warnings present', () => {
    const wrapper = mountFailedChecks({ errors: [], warnings: [] });


Let's go with the destructuring style you use elsewhere in this PR.

Suggested change

const wrapper = mountFailedChecks({ errors: [], warnings: [] });

const { container } = mountFailedChecks({ errors: [], warnings: [] });

This same suggestion also applies to other places where the render() result is used.

eemeli · 2026-01-29T13:16:33Z

translate/src/modules/comments/components/CommentsList.test.js

    });

-    expect(wrapper.find('ul > *')).toHaveLength(3);
+    expect(container.querySelectorAll('ul > *')).toHaveLength(3);


Using querySelector() is a bit of a code smell with testing-library, tbh. It's ok to do so initially here, but it would be good to leave // TODO code comments in the vicinity of each such test to replace them with getBy* and similar testing-library-ish approaches.

eemeli · 2026-01-29T13:27:48Z

translate/src/modules/history/components/History.tsx

-      <section className='history'>
+      <section
+        className='history'
+        data-testid='history-History--no-translations'


You've taken this testid from the localization id. Rather than doing so, this (and potentially other similar cases) seem like a perfect use case for getByText().

eemeli · 2026-01-29T13:31:50Z

translate/src/modules/entitieslist/components/EntitiesList.test.js

@@ -52,7 +53,7 @@ describe('<EntitiesList>', () => {
      hasMore: true,
    });
    const wrapper = mountComponentWithStore(EntitiesList, store);


Breaking this suggestion in two to deal with GitHub's limitations; similar also applies to other tests -- the getBy* functions should be preferred where they're an option, and they don't need an expect() wrapper for an N=1 expected result.

Suggested change

const wrapper = mountComponentWithStore(EntitiesList, store);

const { getByTestId } = mountComponentWithStore(EntitiesList, store);

eemeli · 2026-01-29T13:32:12Z

translate/src/modules/entitieslist/components/EntitiesList.test.js

    });
    const wrapper = mountComponentWithStore(EntitiesList, store);
-    expect(wrapper.find('SkeletonLoader')).toHaveLength(1);
+    expect(wrapper.queryByTestId('skeleton-loader')).toBeInTheDocument();


Suggested change

expect(wrapper.queryByTestId('skeleton-loader')).toBeInTheDocument();

getByTestId('skeleton-loader');

eemeli · 2026-01-29T13:38:17Z

translate/src/modules/navbar/components/Navigation.test.jsx

+    expect(wrapper.container).toHaveTextContent('Klingon');
+    expect(wrapper.container).toHaveTextContent('kg');


All of the tests (not just in this file, but also elsewhere) that use .toHaveTextContent() and other replacements for Enzyme's .text() might be best replaced with getByText(), which also accepts a regexp argument.

eemeli · 2026-01-29T13:40:30Z

translate/src/modules/otherlocales/components/OtherLocaleTranslation.test.jsx


  it('renders a plain translation correctly', () => {
-    const wrapper = createTranslation('', PLAIN_TRANSLATION);
+    createTranslation('', PLAIN_TRANSLATION);


Suggested change

createTranslation('', PLAIN_TRANSLATION);

const { getByText } = createTranslation('', PLAIN_TRANSLATION);

eemeli · 2026-01-29T13:40:59Z

translate/src/modules/otherlocales/components/OtherLocaleTranslation.test.jsx


-    const gt = wrapper.find('GenericTranslation');
-    expect(gt.props().content).toMatch(/^Un cheval/);
+    expect(screen.getByText(/^Un cheval/)).toBeInTheDocument();


Suggested change

expect(screen.getByText(/^Un cheval/)).toBeInTheDocument();

getByText(/^Un cheval/);

eemeli · 2026-01-29T13:47:01Z

translate/src/modules/translationform/components/TranslationForm-multiple.test.jsx

-  const views = Array.from(
-    wrapper.find('.translationform').instance().querySelectorAll('.cm-content'),
-  ).map((el) => el.cmView.view);
+  const form = wrapper.container.querySelector('.translationform');


Wouldn't getByRole('form') work here?

eemeli · 2026-01-29T13:53:03Z

translate/src/modules/translationform/components/TranslationForm-multiple.test.jsx

+    expect(container.querySelectorAll('.accesskeys button')).toHaveLength(8);
+    expect(
+      container.querySelectorAll('.accesskeys button')[0],
+    ).toHaveTextContent('C');
+    expect(
+      container.querySelectorAll('.accesskeys button')[1],
+    ).toHaveTextContent('a');
+    expect(
+      container.querySelectorAll('.accesskeys button')[2],
+    ).toHaveTextContent('n');
+    expect(
+      container.querySelectorAll('.accesskeys button')[3],
+    ).toHaveTextContent('d');
+    expect(
+      container.querySelectorAll('.accesskeys button')[4],
+    ).toHaveTextContent('i');
+    expect(
+      container.querySelectorAll('.accesskeys button')[5],
+    ).toHaveTextContent('t');
+    expect(
+      container.querySelectorAll('.accesskeys button')[6],
+    ).toHaveTextContent('e');
+    expect(
+      container.querySelectorAll('.accesskeys button')[7],
+    ).toHaveTextContent('s');


Something like this would be more legible:

Suggested change

expect(container.querySelectorAll('.accesskeys button')).toHaveLength(8);

expect(

container.querySelectorAll('.accesskeys button')[0],

).toHaveTextContent('C');

expect(

container.querySelectorAll('.accesskeys button')[1],

).toHaveTextContent('a');

expect(

container.querySelectorAll('.accesskeys button')[2],

).toHaveTextContent('n');

expect(

container.querySelectorAll('.accesskeys button')[3],

).toHaveTextContent('d');

expect(

container.querySelectorAll('.accesskeys button')[4],

).toHaveTextContent('i');

expect(

container.querySelectorAll('.accesskeys button')[5],

).toHaveTextContent('t');

expect(

container.querySelectorAll('.accesskeys button')[6],

).toHaveTextContent('e');

expect(

container.querySelectorAll('.accesskeys button')[7],

).toHaveTextContent('s');

const buttonTexts = Array.from(

container.querySelectorAll('.accesskeys button'),

button => button.textContent

);

expect(buttonTexts).toMatchObject(['C', 'a', 'n', 'd', 'i', 't', 'e', 's']);

mathjazz closed this Jan 20, 2026

mathjazz reopened this Jan 20, 2026

mathjazz reviewed Jan 23, 2026

View reviewed changes

nishitmistry force-pushed the shift-mocked-store-enzyme-test-to-rtl branch from 579e89f to 8ba0e27 Compare January 23, 2026 16:44

nishitmistry requested a review from mathjazz January 23, 2026 16:48

nishitmistry marked this pull request as draft January 23, 2026 17:41

mathjazz mentioned this pull request Jan 23, 2026

Added jsdom for vitest #3920

Merged

nishitmistry marked this pull request as ready for review January 25, 2026 09:49

nishitmistry added 2 commits January 25, 2026 15:26

added @testing-library/jest-dom package

1583d24

fixed the broken test after shifting mocked store to rtl

fd687b4

nishitmistry force-pushed the shift-mocked-store-enzyme-test-to-rtl branch from 28d5063 to fd687b4 Compare January 25, 2026 09:56

mathjazz requested review from eemeli and removed request for mathjazz January 29, 2026 12:30

eemeli requested changes Jan 29, 2026

View reviewed changes

		@@ -30,7 +30,9 @@ describe('<FailedChecks>', () => {
		it('does not render if no errors or warnings present', () => {
		const wrapper = mountFailedChecks({ errors: [], warnings: [] });

	const wrapper = mountFailedChecks({ errors: [], warnings: [] });
	const { container } = mountFailedChecks({ errors: [], warnings: [] });

	const wrapper = mountComponentWithStore(EntitiesList, store);
	const { getByTestId } = mountComponentWithStore(EntitiesList, store);

	expect(wrapper.queryByTestId('skeleton-loader')).toBeInTheDocument();
	getByTestId('skeleton-loader');

		expect(wrapper.container).toHaveTextContent('Klingon');
		expect(wrapper.container).toHaveTextContent('kg');

	createTranslation('', PLAIN_TRANSLATION);
	const { getByText } = createTranslation('', PLAIN_TRANSLATION);

	expect(screen.getByText(/^Un cheval/)).toBeInTheDocument();
	getByText(/^Un cheval/);

Shift mocked store enzyme test to React Testing Library #3909

Are you sure you want to change the base?

Shift mocked store enzyme test to React Testing Library #3909

Uh oh!

Conversation

nishitmistry commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mathjazz commented Jan 20, 2026

Uh oh!

nishitmistry commented Jan 21, 2026

Uh oh!

mathjazz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nishitmistry Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathjazz Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nishitmistry Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eemeli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nishitmistry commented Jan 17, 2026 •

edited

Loading

mathjazz left a comment •

edited

Loading

nishitmistry Jan 23, 2026 •

edited

Loading

mathjazz Jan 23, 2026 •

edited

Loading

nishitmistry Jan 23, 2026 •

edited

Loading