diff --git a/VAMobile/documentation/docs/Engineering/FrontEnd/Testing/UnitTesting.md b/VAMobile/documentation/docs/Engineering/FrontEnd/Testing/UnitTesting.md index b582d5f5bee..02666077491 100644 --- a/VAMobile/documentation/docs/Engineering/FrontEnd/Testing/UnitTesting.md +++ b/VAMobile/documentation/docs/Engineering/FrontEnd/Testing/UnitTesting.md @@ -17,33 +17,61 @@ All React components should have at least one unit test. The ideal quantity of t Note that while a high coverage percentage is good, it doesn't ensure tests are complete and correct. It's important to think critically and implement tests that cover the key cases a user might encounter. ### More information + - Google's [Code Coverage Best Practices](https://testing.googleblog.com/2020/08/code-coverage-best-practices.html) - [How to know what to test](https://kentcdodds.com/blog/how-to-know-what-to-test) (Kent C Dodds) ## Targeting by rendered text, label, or role ❌ Avoid targeting child props based on numeric order: + ```tsx -expect(textView[5].props.children).toEqual('Rx #: 3636691') +expect(textView[5].props.children).toEqual(`${t('prescription.rxNumber')}: 3636691`) ``` -✅ Instead, target rendered text, role, or accessibility label: +✅ Instead, target rendered text, accessibility label, or role: ```tsx -expect(screen.getByText('Rx #: 3636691')).toBeTruthy() -expect(screen.getByLabelText('Prescription number 3636691')).toBeTruthy() -expect(screen.getByRole('checkbox', { name: 'Prescription 1 of 3', checked: true })).toBeTruthy() +expect(screen.getByText(`${t('prescription.rxNumber')}: 3636691`)).toBeTruthy() +expect(screen.getByLabelText(`${t('prescription.rxNumber.a11yLabel')} 3636691`)).toBeTruthy() +expect( + screen.getByRole('checkbox', { + name: t('prescription.history.orderIdentifier', { idx: 1, total: 3 }), + checked: true, + }), +).toBeTruthy() ``` ### Why? -This method reduces test fragility because moving an element into/out of a child component, changing props, or adding/removing sibling components does not break the test. Targeting accessibility label or role ensures screen readers read the correct label and/or role to the user, preventing a11y regressions. Finally, this type of test is simpler to read and write because it ignores implementation details, focusing instead on what the user expects to see in rendered output. + +This method reduces test fragility because moving an element into/out of a child component, changing props, or adding/removing sibling components does not break the test. Targeting accessibility label or role ensures screen readers read the correct label and/or role to the user, preventing a11y regressions. Finally, this type of test is simpler to read and write because it ignores implementation details, focusing instead on what the user expects to see in rendered output. ### More information + - React Testing Library's [guiding principles](https://testing-library.com/docs/guiding-principles) - [Some thoughts](https://www.boyney.io/blog/2019-05-21-my-experience-moving-from-enzyme-to-react-testing-library) on why this RTL approach is an improvement over Enzyme - [The Dangers of Shallow Rendering](https://mskelton.medium.com/the-dangers-of-shallow-rendering-343e48fe5f28) +## Targeting by translated text + +❌ Avoid using actual strings to target elements: + +```tsx +fireEvent.press(screen.getByRole('tab', { name: "Make sure you're in the right health portal" })) +``` + +✅ Instead, call the translation function as you do in the component under test: + +```tsx +fireEvent.press(screen.getByRole('tab', { name: t('cernerAlertSM.header') })) +``` + +### Why? + +Using the translation function reduces test fragility. Minor wording changes won't break the test. + ## Firing events + ❌ Avoid calling a callback function in a prop to simulate user interaction: ```tsx @@ -53,33 +81,39 @@ testInstance.findByType(Pressable).props.onPress() ✅ Instead, fire a press event: ```tsx -fireEvent.press(screen.getByText('Cancel')) +fireEvent.press(screen.getByText(t('cancel'))) ``` ✅ Fire a changeText event: ```tsx -fireEvent.changeText(screen.getByText('Phone'), '123-456-7890'); +fireEvent.changeText(screen.getByText(t('phone')), '123-456-7890') ``` ✅ Fire a scroll event: ```tsx fireEvent.scroll(screen.getByText('scroll-view'), { - nativeEvent: { contentOffset: { y: 200 } } + nativeEvent: { contentOffset: { y: 200 } }, }) ``` ### Why? + Calling a callback function in a prop only checks that the function runs. It doesn’t test that the element is visible to the user and that it’s wired up correctly. It’s also fragile because refactoring the component might change the props and break the test. Firing an event resolves these concerns, which also apply to text fields and scrolling. ## Exercising key functionality -❌ Avoid tests that just check large quantities of static props: + +❌ Avoid tests that just check large quantities of static text: ```tsx expect(textView[6].props.children).toEqual('What’s next') -expect(textView[7].props.children).toEqual("We're reviewing your refill request. Once approved, the VA pharmacy will process your refill.") -expect(textView[8].props.children).toEqual('If you have questions about the status of your refill, contact your provider or local VA pharmacy.') +expect(textView[7].props.children).toEqual( + "We're reviewing your refill request. Once approved, the VA pharmacy will process your refill.", +) +expect(textView[8].props.children).toEqual( + 'If you have questions about the status of your refill, contact your provider or local VA pharmacy.', +) ``` ✅ Instead, focus on tests that check important functionality: @@ -87,7 +121,7 @@ expect(textView[8].props.children).toEqual('If you have questions about the stat ```tsx describe('on click of the "Go to inbox" link', () => { it('calls useRouteNavigation and updateSecureMessagingTab', () => { - fireEvent.press(screen.getByRole('link', { name: 'Go to inbox' })) + fireEvent.press(screen.getByRole('link', { name: t('secureMessaging.goToInbox') })) expect(navigate).toHaveBeenCalled() expect(updateSecureMessagingTab).toHaveBeenCalled() }) @@ -95,29 +129,34 @@ describe('on click of the "Go to inbox" link', () => { ``` ### Why? + Each test should add value by serving as a focused warning that something important has failed. Testing that a sequence of TextViews renders certain text doesn't tell us much. It's also fragile because the smallest text change breaks the test. Testing important and/or complex logic is more beneficial because that’s where high-impact regressions typically occur. In addition, tests for complicated logic serve as a form of documentation, letting engineers know how the code is supposed to function. ### More information + - See #2 of [The 7 Sins of Unit Testing](https://www.testrail.com/blog/the-7-sins-of-unit-testing/) about why more assertions can be worse, not better ## Testing from the user’s perspective + Consider what the user expects to do and see, then write tests that simulate it. For example, let's say the user expects to press “Select all”, then see two checked checkboxes and relevant text. ✅ This test tells the user's story and checks it at the same time: ```tsx it('toggles items when "Select all" is pressed', () => { - fireEvent.press(screen.getByText('Select all')) - expect(screen.getByRole('checkbox', { name: 'One', checked: true })).toBeTruthy() - expect(screen.getByRole('checkbox', { name: 'Two', checked: true })).toBeTruthy() - expect(screen.getByText('2/2 selected')).toBeTruthy() + fireEvent.press(screen.getByText(t('select.all'))) + expect(screen.getByRole('checkbox', { name: t('one'), checked: true })).toBeTruthy() + expect(screen.getByRole('checkbox', { name: t('two'), checked: true })).toBeTruthy() + expect(screen.getByText(t('selectedOutOfTotal', { selected: 2, total: 2 }))).toBeTruthy() }) ``` ### Why? + By taking the user's point of view, user-focused tests help prevent the most damaging regressions, ones which prevent users from completing their desired tasks. But because implementation details aren't baked into the test, engineers retain the flexibility to refactor as needed without causing test failures. ### More information + - Why it's important to focus on the [end user](https://kentcdodds.com/blog/avoid-the-test-user) and avoid the "test user" ## Test File Naming @@ -127,6 +166,7 @@ The test file should live in the same location as its associated component with `ClaimsScreen.tsx` will have a test file named `ClaimsScreen.test.tsx` ## Running Tests + - Run unit tests with `yarn test` - Coverage can be found under `coverage/lcov-report/index.html` @@ -174,12 +214,12 @@ This block of code will mock the entirety of the hooks util file using the origi navigateToPaymentMissingSpy = jest.fn() when(mockNavigationSpy) - .mockReturnValue(() => {}) - .calledWith('PaymentMissing') - .mockReturnValue(navigateToPaymentMissingSpy) + .mockReturnValue(() => {}) + .calledWith('PaymentMissing') + .mockReturnValue(navigateToPaymentMissingSpy) ``` -This will create another object `navigateToPaymentMissingSpy` that will be returned if the hook is called with the parameters `'PaymentMissing'` +This will create another object `navigateToPaymentMissingSpy` that will be returned if the hook is called with the parameters `'PaymentMissing'` ```tsx // Do something that will trigger a navigation to the PaymentMissing screen @@ -212,4 +252,4 @@ const letterOptions = { expect(downloadLetter).toBeCalledWith(LetterTypeConstants.benefitSummary, letterOptions) ``` -This checks to see that the `downloadLetter` action was called with the expected parameters \ No newline at end of file +This checks to see that the `downloadLetter` action was called with the expected parameters diff --git a/VAMobile/src/screens/BenefitsScreen/BenefitsScreen.test.tsx b/VAMobile/src/screens/BenefitsScreen/BenefitsScreen.test.tsx index 21bf6fb0655..563a4f0f6e6 100644 --- a/VAMobile/src/screens/BenefitsScreen/BenefitsScreen.test.tsx +++ b/VAMobile/src/screens/BenefitsScreen/BenefitsScreen.test.tsx @@ -1,6 +1,7 @@ import React from 'react' import { fireEvent, screen, waitFor } from '@testing-library/react-native' +import { t } from 'i18next' import { DateTime } from 'luxon' import { claimsAndAppealsKeys } from 'api/claimsAndAppeals' @@ -45,19 +46,23 @@ context('BenefitsScreen', () => { it('displays active claims count when veteran has active claims', () => { const activeClaimsCount = 3 initializeTestInstance(activeClaimsCount) - expect(screen.getByRole('link', { name: 'Claims' })).toBeTruthy() - expect(screen.getByRole('link', { name: `${activeClaimsCount} active` })).toBeTruthy() + expect(screen.getByRole('link', { name: t('claims.title') })).toBeTruthy() + expect( + screen.getByRole('link', { name: t('claims.activityButton.subText', { count: activeClaimsCount }) }), + ).toBeTruthy() }) it('does not display active claims count when there are no active claims', () => { const activeClaimsCount = 0 initializeTestInstance(activeClaimsCount) - expect(screen.queryByRole('link', { name: `${activeClaimsCount} active` })).toBeFalsy() + expect( + screen.queryByRole('link', { name: t('claims.activityButton.subText', { count: activeClaimsCount }) }), + ).toBeFalsy() }) it('navigates to Claims history screen when claims button is pressed', () => { initializeTestInstance(3) - fireEvent.press(screen.getByRole('link', { name: 'Claims' })) + fireEvent.press(screen.getByRole('link', { name: t('claims.title') })) expect(mockNavigationSpy).toHaveBeenCalledWith('ClaimsHistoryScreen') }) @@ -71,7 +76,9 @@ context('BenefitsScreen', () => { ] initializeTestInstance(activeClaimsCount, serviceErrors) - expect(screen.queryByRole('link', { name: `${activeClaimsCount} active` })).toBeFalsy() + expect( + screen.queryByRole('link', { name: t('claims.activityButton.subText', { count: activeClaimsCount }) }), + ).toBeFalsy() }) it('displays warning message when there is a service error', () => { @@ -84,11 +91,7 @@ context('BenefitsScreen', () => { ] initializeTestInstance(activeClaimsCount, serviceErrors) - expect( - screen.getByText( - "We can't get some of your information right now. Benefits activity may not be accurate. Check back later.", - ), - ).toBeTruthy() + expect(screen.getByText(t('benefits.activity.nonFatalError'))).toBeTruthy() }) it('displays downtime message when claims or appeals is in downtime', () => { @@ -107,9 +110,7 @@ context('BenefitsScreen', () => { } initializeTestInstance(activeClaimsCount, undefined, preloadedState) - expect( - screen.getByText("We're working on the mobile app. Benefits activity may not be accurate. Check back later."), - ).toBeTruthy() + expect(screen.getByText(t('benefits.activity.warning.downtime'))).toBeTruthy() }) it('does not display active claims when claims or appeals is in downtime', () => { @@ -128,7 +129,9 @@ context('BenefitsScreen', () => { } initializeTestInstance(activeClaimsCount, undefined, preloadedState) - expect(screen.queryByRole('link', { name: `${activeClaimsCount} active` })).toBeFalsy() + expect( + screen.queryByRole('link', { name: t('claims.activityButton.subText', { count: activeClaimsCount }) }), + ).toBeFalsy() }) it('displays error message when the claims API call throws an error', async () => { @@ -136,11 +139,7 @@ context('BenefitsScreen', () => { .calledWith('/v0/claims-and-appeals-overview', expect.anything()) .mockRejectedValue('fail') initializeTestInstance() - await waitFor(() => - expect( - screen.getByText("We can't get some of your information. Benefits activity may not be accurate."), - ).toBeTruthy(), - ) + await waitFor(() => expect(screen.getByText(t('benefits.activity.error'))).toBeTruthy()) }) it('does not display active claims count when the claims API call throws an error', async () => { @@ -148,23 +147,19 @@ context('BenefitsScreen', () => { .calledWith('/v0/claims-and-appeals-overview', expect.anything()) .mockRejectedValue('fail') initializeTestInstance() - await waitFor(() => - expect( - screen.getByText("We can't get some of your information. Benefits activity may not be accurate."), - ).toBeTruthy(), - ) - await waitFor(() => expect(screen.queryByText('active')).toBeFalsy()) + await waitFor(() => expect(screen.getByText(t('benefits.activity.error'))).toBeTruthy()) + await waitFor(() => expect(screen.queryByText(t('active'))).toBeFalsy()) }) it("navigates to Letters screen when 'letters and document' button is pressed", () => { initializeTestInstance() - fireEvent.press(screen.getByRole('link', { name: 'VA letters and documents' })) + fireEvent.press(screen.getByRole('link', { name: t('lettersAndDocs.title') })) expect(mockNavigationSpy).toHaveBeenCalledWith('LettersOverview') }) it('navigates to Disability rating screen when Disability rating button is pressed', () => { initializeTestInstance() - fireEvent.press(screen.getByRole('link', { name: 'Disability rating' })) + fireEvent.press(screen.getByRole('link', { name: t('disabilityRating.title') })) expect(mockNavigationSpy).toHaveBeenCalledWith('DisabilityRatings') }) }) diff --git a/VAMobile/src/screens/HealthScreen/CernerAlert/CernerAlert.test.tsx b/VAMobile/src/screens/HealthScreen/CernerAlert/CernerAlert.test.tsx index 528c682d275..e03d07d48c3 100644 --- a/VAMobile/src/screens/HealthScreen/CernerAlert/CernerAlert.test.tsx +++ b/VAMobile/src/screens/HealthScreen/CernerAlert/CernerAlert.test.tsx @@ -2,8 +2,10 @@ import React from 'react' import { Alert } from 'react-native' import { fireEvent, screen } from '@testing-library/react-native' +import { t } from 'i18next' import { context, render } from 'testUtils' +import { a11yLabelVA } from 'utils/a11yLabel' import CernerAlert from './CernerAlert' @@ -68,37 +70,25 @@ context('CernerAlert', () => { }) it('When only cerner facilities', () => { - fireEvent.press(screen.getByRole('tab', { name: 'Your care team uses My VA Health' })) - expect(screen.getByLabelText('Your care team uses My V-A Health')).toBeTruthy() - expect( - screen.getByLabelText("You'll need to use our My V-A Health portal to manage your care at these facilities:"), - ).toBeTruthy() - expect( - screen.getByText("You'll need to use our My VA Health portal to manage your care at these facilities:"), - ).toBeTruthy() + fireEvent.press(screen.getByRole('tab', { name: t('healthHelp.usesVAHealth') })) + expect(screen.getByLabelText(a11yLabelVA(t('healthHelp.usesVAHealth')))).toBeTruthy() + expect(screen.getByLabelText(a11yLabelVA(t('healthHelp.goToPortal')))).toBeTruthy() + expect(screen.getByText(t('healthHelp.goToPortal'))).toBeTruthy() expect(screen.getByText('FacilityOne')).toBeTruthy() expect(screen.getByText('FacilityTwo')).toBeTruthy() - fireEvent.press(screen.getByRole('link', { name: 'Go to My VA Health' })) + fireEvent.press(screen.getByRole('link', { name: t('goToMyVAHealth') })) expect(Alert.alert).toBeCalled() }) it('when some facilities are cerner and pressing the link', async () => { - fireEvent.press(screen.getByRole('tab', { name: 'Some of your care team uses My VA Health' })) - expect(screen.getByLabelText('Some of your care team uses My V-A Health')).toBeTruthy() - expect( - screen.getByLabelText("You'll need to use our My V-A Health portal to manage your care at these facilities:"), - ).toBeTruthy() - expect( - screen.getByText("You'll need to use our My VA Health portal to manage your care at these facilities:"), - ).toBeTruthy() + fireEvent.press(screen.getByRole('tab', { name: t('cernerAlert.header.some') })) + expect(screen.getByLabelText(a11yLabelVA(t('cernerAlert.header.some')))).toBeTruthy() + expect(screen.getByLabelText(a11yLabelVA(t('healthHelp.goToPortal')))).toBeTruthy() + expect(screen.getByText(t('healthHelp.goToPortal'))).toBeTruthy() expect(screen.getByText('FacilityOne')).toBeTruthy() - expect( - screen.getByLabelText('You can still use this app to manage your care at other V-A health facilities.'), - ).toBeTruthy() - expect( - screen.getByText('You can still use this app to manage your care at other VA health facilities.'), - ).toBeTruthy() - fireEvent.press(screen.getByLabelText('Go to My V-A Health')) + expect(screen.getByLabelText(a11yLabelVA(t('healthHelp.canStillUse')))).toBeTruthy() + expect(screen.getByText(t('healthHelp.canStillUse'))).toBeTruthy() + fireEvent.press(screen.getByLabelText(a11yLabelVA(t('goToMyVAHealth')))) expect(Alert.alert).toBeCalled() }) }) diff --git a/VAMobile/src/screens/HealthScreen/SecureMessaging/TermsAndConditions/TermsAndConditions.test.tsx b/VAMobile/src/screens/HealthScreen/SecureMessaging/TermsAndConditions/TermsAndConditions.test.tsx index 26b228ff3f4..48e1c4b33aa 100644 --- a/VAMobile/src/screens/HealthScreen/SecureMessaging/TermsAndConditions/TermsAndConditions.test.tsx +++ b/VAMobile/src/screens/HealthScreen/SecureMessaging/TermsAndConditions/TermsAndConditions.test.tsx @@ -2,6 +2,7 @@ import React from 'react' import { Alert } from 'react-native' import { fireEvent, screen } from '@testing-library/react-native' +import { t } from 'i18next' import { context, render } from 'testUtils' @@ -13,21 +14,15 @@ context('TermsAndConditions', () => { }) it('initializes correctly', () => { - expect(screen.getByText('You’re required to accept the current terms and conditions')).toBeTruthy() - expect( - screen.getByText('To accept the current terms and conditions, please go to the My HealtheVet website: '), - ).toBeTruthy() - expect(screen.getByRole('link', { name: 'Go to My HealtheVet' })).toBeTruthy() - expect( - screen.getByText( - 'Note: Do not use Secure Messaging if you have a medical emergency or an urgent need. It may take a few days for you to get a reply.', - ), - ).toBeTruthy() + expect(screen.getByText(t('termsAndConditions.title'))).toBeTruthy() + expect(screen.getByText(t('termsAndConditions.toAccept'))).toBeTruthy() + expect(screen.getByRole('link', { name: t('termsAndConditions.goTo') })).toBeTruthy() + expect(screen.getByText(t('secureMessaging.doNotUseSM'))).toBeTruthy() }) describe('when Go to My HealtheVet link is clicked', () => { it('should launch external link', () => { - fireEvent.press(screen.getByRole('link', { name: 'Go to My HealtheVet' })) + fireEvent.press(screen.getByRole('link', { name: t('termsAndConditions.goTo') })) expect(Alert.alert).toBeCalled() }) })