test #174: created unit test for Avatar component by jefische · Pull Request #637 · LetsGetTechnical/elecretanta

jefische · 2025-10-16T15:17:58Z

Description

Before:

The Avatar component lacked unit tests.

After:

Unit tests have been added for `Avatar` component:

renders user avatar image when valid src attribute is provided
renders fallback/default avatar image when src is undefined
added alt text to AvatarImage to access element by semantic query

Closes #174

Testing instructions

If applicable, provide steps for reviewers to test changes locally -- including necessary setup, commands, and expected results

Additional information

Share any additional info that may provide context for the PR evaluation (performance considerations, design choices, etc)

[optional] Screenshots

Pre-submission checklist

Code builds and passes locally
PR title follows Conventional Commit format (e.g. test #001: created unit test for __ component)
Request reviews from the Peer Code Reviewers and Senior+ Code Reviewers groups
Thread has been created in Discord and PR is linked in gis-code-questions

vercel · 2025-10-16T15:18:04Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
elecretanta	Ready	Preview	Comment	Nov 5, 2025 4:58pm
elecretanta-storybook	Ready	Preview	Comment	Nov 5, 2025 4:58pm
elecretanta-unit-test	Ready	Preview	Comment	Nov 5, 2025 4:58pm

suzuhe481 · 2025-10-19T01:20:07Z

+  const actual = jest.requireActual('@radix-ui/react-avatar');
+  return ({
+    ...actual,
+    Image: ({src, alt, ...props}: {src?: string, alt?: string}) => {


Remove ...props from the Image mock. When testing, you should only mock the necessary behavior. For the Avatar component, you're currently only testing for src and alt. The other properties of Image aren't needed.

suzuhe481 · 2025-10-19T01:32:18Z

+  return ({
+    ...actual,
+    Image: ({src, alt, ...props}: {src?: string, alt?: string}) => {
+      return <img data-testid="avatar-image" src={src} alt={alt} {...props}/>;


Don't add data-testids inside components used in testing. While you fetched the element by role, which is good, the id still isn't necessary for the mock.

If anything, this id should have been added to the AvatarImage component.

suzuhe481 · 2025-10-19T01:35:32Z

+  const actual = jest.requireActual('@radix-ui/react-avatar');
+  return ({
+    ...actual,
+    Image: ({src, alt, ...props}: {src?: string, alt?: string}) => {


Remove the optional ? on the alt?. This property will always be included in both the AvatarImage and AvatarFallback.

nickytonline · 2025-10-19T03:26:32Z

  return (
    <AvatarBody>
-      <AvatarImage src={userAvatar} alt="" />
+      <AvatarImage src={userAvatar} alt="user avatar" />


refactor (a11y): "user avatar" is too generic. If the avatar conveys meaning, add more context, like the username, otherwise leave the alt test to "".

Suggested change

<AvatarImage src={userAvatar} alt="user avatar" />

<AvatarImage src={userAvatar} alt=`profile image for user ${username}` />

I think adding more context with the username makes sense. After adding username as a prop:

const Avatar = ({ userAvatar, username }: { userAvatar: string | undefined, username: string })

I believe I would need to update each instance where the Avatar component is used:

elecretanta/components/UserDropdownMenu/UserDropdownMenu.tsx

Line 43 in 0a2d352

<Avatar userAvatar={avatar} />

elecretanta/components/ProfileCard/ProfileCard.tsx

Line 92 in 0a2d352

<Avatar data-testid="avatar" userAvatar={profile?.avatar} />

elecretanta/components/MembersList/MembersList.tsx

Line 31 in 0a2d352

<Avatar userAvatar={member.member.avatar} />

I want to make sure this correct and okay to do inside the same PR as I'm starting to change multiple files. For the UserDropdownMenu component it gets more complicated to pass a username as well.

Also, this is only a unit test task. This should be removed into an A11Y ticket. I want to keep things simple for your first test.

Thanks, I'll just leave the AvatarImage alt text as it was prior to my edits.

nickytonline · 2025-10-19T03:41:11Z

+import Avatar from './Avatar';
+import { render, screen } from '@testing-library/react';
+
+jest.mock('@radix-ui/react-avatar', () => {


Just FYI for anyone coming across this. My first instinct was to ask why are we mocking this as you always want to mock as little as possible or not at all so we are testing the actual implementation.

In this case, we mock Radix’s Avatar.Image because in JSDOM, what we use in Jest tests, image load events don’t fire as it's not a real browser environment, so only the fallback is ever shown in tests if we don't mock.

shashilo · 2025-10-22T02:07:16Z

  return (
    <AvatarBody>
-      <AvatarImage src={userAvatar} alt="" />
+      <AvatarImage src={userAvatar} alt="user avatar" />


Also, this is only a unit test task. This should be removed into an A11Y ticket. I want to keep things simple for your first test.

shashilo · 2025-10-22T02:35:39Z

+
+  });
+
+  it('Renders the fallback image when src is undefined', () => {


What about when there's a string given bug the image is not found?

That makes sense, I'll add this test scenario.

shashilo · 2025-10-22T02:36:01Z

+
+describe('Avatar Component', () => {
+
+  it('Renders the component with user image using valid src attribute', () => {


Should the fallback component show here?

Not for this scenario. I wanted to test that the user provided image is correctly added to the document.

Add an exception to check that fallback doesn't show in this default scenario.

Interestingly the fallback image is still showing in the document for this scenario. I tried using .not.toBeVisible() method as well as .not.toBeInTheDocument() but both tests fail:

1st attempt:

+ const avatarElementFallbackImage = screen.getByTestId('avatar-fallback') + expect(avatarElementFallbackImage).not.toBeVisible();

2nd attempt:

+ const avatarElementFallbackImage = screen.getByTestId('avatar-fallback') + expect(avatarElementFallbackImage).not.toBeInTheDocument();

I tried looking at screen.debug() and it's showing both elements in the JSDOM. Are both of these supposed to show up like this?

shashilo · 2025-10-22T02:38:08Z

+  it('Renders the component with user image using valid src attribute', () => {
+    render(<Avatar userAvatar='https://mdbcdn.b-cdn.net/img/new/avatars/2.webp' />);
+
+    const avatarElementWithImage = screen.getByRole('img', { name: 'user avatar'});


Follow how we're doing the other components by adding a data-testid attribute.

nickytonline

🚀

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

jefische · 2025-11-05T17:02:44Z

The unit test for the ProfileCard component failed as a result of adding data-testid="avatar-image" to the AvatarImage component. ProfileCard was mocking the image and adding a data-testid to the mock rather than the actual component. They were also mocking the AvatarFallback incorrectly so I’ve updated the ProfileCard test to be consistent with the mock for our Avatar component. All tests are passing now.

jefische added 2 commits October 14, 2025 15:51

test: #174 initial unit test for Avatar component

a13cac1

adding test scenario and alt text for AvatarImage

b3f2662

jefische requested review from HoldUpFjord, alexappleget, bethanyann, jennifertieu, kepsteen, nickytonline and shashilo as code owners October 16, 2025 15:17

jefische changed the title ~~Jeremy/174 unit test avatar component~~ test #174: created unit test for Avatar component Oct 16, 2025

jefische requested review from a team October 16, 2025 16:30

suzuhe481 reviewed Oct 19, 2025

View reviewed changes

nickytonline requested changes Oct 19, 2025

View reviewed changes

Merge branch 'develop' into jeremy/174-unit-test-avatar-component

2b12edc

vercel Bot had a problem deploying to Preview – elecretanta-unit-test October 20, 2025 18:29 Failure

vercel Bot deployed to Preview – elecretanta-storybook October 20, 2025 18:30 View deployment

vercel Bot had a problem deploying to Preview – elecretanta October 20, 2025 18:31 Failure

shashilo requested changes Oct 22, 2025

View reviewed changes

Revise tests for Avatar component after review

d87c46f

shashilo requested review from a team October 22, 2025 15:13

vercel Bot deployed to Preview – elecretanta October 22, 2025 15:14 View deployment

vercel Bot deployed to Preview – elecretanta-storybook October 22, 2025 15:15 View deployment

vercel Bot had a problem deploying to Preview – elecretanta-unit-test October 22, 2025 15:15 Failure

Merge branch 'develop' into jeremy/174-unit-test-avatar-component

d7931ee

shashilo removed the request for review from a team October 22, 2025 15:28

shashilo requested review from a team and removed request for a team October 22, 2025 15:28

vercel Bot deployed to Preview – elecretanta October 22, 2025 15:29 View deployment

vercel Bot deployed to Preview – elecretanta-storybook October 22, 2025 15:30 View deployment

vercel Bot had a problem deploying to Preview – elecretanta-unit-test October 22, 2025 15:30 Failure

shashilo previously approved these changes Oct 22, 2025

View reviewed changes

Merge branch 'develop' into jeremy/174-unit-test-avatar-component

009fece

vercel Bot deployed to Preview – elecretanta-storybook October 24, 2025 16:21 View deployment

vercel Bot deployed to Preview – elecretanta October 24, 2025 16:21 View deployment

vercel Bot had a problem deploying to Preview – elecretanta-unit-test October 24, 2025 16:22 Failure

nickytonline previously approved these changes Oct 25, 2025

View reviewed changes

Merge branch 'develop' into jeremy/174-unit-test-avatar-component

faafeea

vercel Bot had a problem deploying to Preview – elecretanta-unit-test October 30, 2025 02:17 Failure

vercel Bot deployed to Preview – elecretanta-storybook October 30, 2025 02:17 View deployment

vercel Bot deployed to Preview – elecretanta October 30, 2025 02:18 View deployment

Merge branch 'develop' into jeremy/174-unit-test-avatar-component

82ab88f

Copilot AI review requested due to automatic review settings November 5, 2025 16:00

vercel Bot had a problem deploying to Preview – elecretanta-unit-test November 5, 2025 16:00 Failure

vercel Bot deployed to Preview – elecretanta-storybook November 5, 2025 16:01 View deployment

vercel Bot deployed to Preview – elecretanta November 5, 2025 16:01 View deployment

Copilot AI reviewed Nov 5, 2025

View reviewed changes

address avatar mocking issues for ProfileCard component

bf87659

jefische dismissed stale reviews from shashilo and nickytonline via bf87659 November 5, 2025 16:56

vercel Bot deployed to Preview – elecretanta-storybook November 5, 2025 16:57 View deployment

vercel Bot deployed to Preview – elecretanta November 5, 2025 16:57 View deployment

vercel Bot deployed to Preview – elecretanta-unit-test November 5, 2025 16:58 View deployment

	<AvatarImage src={userAvatar} alt="user avatar" />
	<AvatarImage src={userAvatar} alt=`profile image for user ${username}` />


		});

		it('Renders the fallback image when src is undefined', () => {


		describe('Avatar Component', () => {

		it('Renders the component with user image using valid src attribute', () => {

Conversation

jefische commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Before:

After:

Unit tests have been added for Avatar component:

Testing instructions

Additional information

[optional] Screenshots

Pre-submission checklist

Uh oh!

vercel Bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

nickytonline left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

jefische commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jefische commented Oct 16, 2025 •

edited

Loading

Unit tests have been added for `Avatar` component:

vercel Bot commented Oct 16, 2025 •

edited

Loading