Skip to content

migrate l10n, /placeable, /project, /resource, /search, /user tests to RTL (partial)#4200

Merged
eemeli merged 3 commits into
mozilla:mainfrom
nishitmistry:migrate-l10n,placeable,project,resource,search,user-test-to-RTL
Jun 9, 2026
Merged

migrate l10n, /placeable, /project, /resource, /search, /user tests to RTL (partial)#4200
eemeli merged 3 commits into
mozilla:mainfrom
nishitmistry:migrate-l10n,placeable,project,resource,search,user-test-to-RTL

Conversation

@nishitmistry

Copy link
Copy Markdown
Collaborator

Continuation of previous MR #4023 the migration of enzyme test to @testing-library/react under task #3767

Some of the tests under /project, /search, and /user have not yet been migrated due to their complexity.

@nishitmistry nishitmistry requested a review from eemeli June 8, 2026 19:41

@eemeli eemeli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, see inline for a few small questions/fixes.

Comment on lines +26 to +27
expect(container.querySelector('span.path')).toBeInTheDocument();
expect(container.querySelector('span.percent')).toBeInTheDocument();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The previous test checked that there was only one span. Is the behaviour now the same, or are there now two spans? If there's only one, shouldn't we check for span.path.percent?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Previously too we were checking for the presence of two spans, one in ResourceItem.tsx and other one hidden by enzyme's shallow render within ResourcePercent component.
expect(wrapper.find('span')).toHaveLength(1); expect(wrapper.find(ResourcePercent)).toHaveLength(1);

now that i look at it again i think we can replace the queryselector with rtl approach

Comment on lines +442 to +444
<Localized
id='search-FiltersPanel--toggle-filters-panel'
attrs={{ 'aria-label': true }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Comment thread translate/src/modules/user/components/UserNotification.test.jsx Outdated
Comment thread translate/src/modules/user/components/UserNotificationsMenu.test.jsx Outdated
return user?.isAuthenticated ? (
<div className='user-notifications-menu'>
<div className='selector' onClick={handleClick}>
<div className='selector' role='button' onClick={handleClick}>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

- Updated ResourceItem tests to include additional props and verify rendered output.
- Refactored UserNotification tests to use getByText for better readability and accuracy in assertions.
- Standardized variable naming for consistency in UserNotificationsMenu tests.
@nishitmistry nishitmistry requested a review from eemeli June 9, 2026 12:42
@eemeli eemeli merged commit f70bf0b into mozilla:main Jun 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants