From c7272d5cf18f89a5b8ec02118ffd2dc3de99ae26 Mon Sep 17 00:00:00 2001 From: Frankie Moran Date: Tue, 31 Mar 2026 13:57:56 +0100 Subject: [PATCH 1/5] feat: add FinesSaPayloadService and transform payload functionality --- .vscode/extensions.json | 3 +- ...nes-sa-defendant-accounts.resolver.spec.ts | 26 ++++++++++++- .../fines-sa-defendant-accounts.resolver.ts | 28 ++++++++----- ...ines-sa-transform-items-config.constant.ts | 15 +++++++ .../services/fines-sa-payload.service.spec.ts | 39 +++++++++++++++++++ .../services/fines-sa-payload.service.ts | 27 +++++++++++++ src/test-setup.ts | 10 +++++ vitest.config.ts | 6 +++ 8 files changed, 143 insertions(+), 11 deletions(-) create mode 100644 src/app/flows/fines/fines-sa/services/constants/fines-sa-transform-items-config.constant.ts create mode 100644 src/app/flows/fines/fines-sa/services/fines-sa-payload.service.spec.ts create mode 100644 src/app/flows/fines/fines-sa/services/fines-sa-payload.service.ts diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 7d8f321a63..761ded21ff 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -24,6 +24,7 @@ "mrmlnc.vscode-scss", "pflannery.vscode-versionlens", "deque-systems.vscode-axe-linter", - "sonarsource.sonarlint-vscode" + "sonarsource.sonarlint-vscode", + "vitest.explorer", ] } diff --git a/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.spec.ts b/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.spec.ts index fe1826572f..6897967b2c 100644 --- a/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.spec.ts +++ b/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.spec.ts @@ -7,6 +7,9 @@ import { FinesSaStoreType } from '../../../stores/types/fines-sa.type'; import { ResolveFn } from '@angular/router'; import { FINES_SA_SEARCH_ACCOUNT_STATE } from '../../../fines-sa-search/fines-sa-search-account/constants/fines-sa-search-account-state.constant'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { FinesSaPayloadService } from '../../../services/fines-sa-payload.service'; +import { FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG } from '../../../services/constants/fines-sa-transform-items-config.constant'; +import { IFinesSaSearchAccountFormIndividualsState } from '../../../fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-individuals/interfaces/fines-sa-search-account-form-individuals-state.interface'; // eslint-disable-next-line @typescript-eslint/no-explicit-any const execIndividuals: ResolveFn = (...params) => @@ -18,6 +21,7 @@ const execCompanies: ResolveFn = (...params) => describe('finesSaDefendantAccountsResolver (store-driven)', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any let opalFines: any; + let finesSaPayloadService: FinesSaPayloadService; let finesSaStore: FinesSaStoreType; beforeEach(() => { @@ -29,12 +33,14 @@ describe('finesSaDefendantAccountsResolver (store-driven)', () => { getDefendantAccounts: vi.fn().mockName('OpalFines.getDefendantAccounts'), }, }, + FinesSaPayloadService, FinesSaStore, ], }); // eslint-disable-next-line @typescript-eslint/no-explicit-any opalFines = TestBed.inject(OpalFines) as any; + finesSaPayloadService = TestBed.inject(FinesSaPayloadService); finesSaStore = TestBed.inject(FinesSaStore); }); @@ -96,6 +102,8 @@ describe('finesSaDefendantAccountsResolver (store-driven)', () => { }); it('builds individual payload when activeTab = individuals and criteria present', async () => { + const transformPayloadSpy = vi.spyOn(finesSaPayloadService, 'transformPayload'); + finesSaStore.setSearchAccount({ ...FINES_SA_SEARCH_ACCOUNT_STATE, fsa_search_account_business_unit_ids: [65, 66, 73, 77, 80, 78], @@ -104,7 +112,7 @@ describe('finesSaDefendantAccountsResolver (store-driven)', () => { fsa_search_account_individuals_last_name_exact_match: true, fsa_search_account_individuals_first_names: 'Jane', fsa_search_account_individuals_first_names_exact_match: false, - fsa_search_account_individuals_date_of_birth: '1980-01-02', + fsa_search_account_individuals_date_of_birth: '02/01/1980', fsa_search_account_individuals_national_insurance_number: 'QQ123456C', fsa_search_account_individuals_address_line_1: '10 Lane', fsa_search_account_individuals_post_code: 'AB1 2CD', @@ -117,6 +125,19 @@ describe('finesSaDefendantAccountsResolver (store-driven)', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any await lastValueFrom(execIndividuals(undefined as any, undefined as any) as Observable); + const transformedCriteria = transformPayloadSpy.mock.calls[0][0] as IFinesSaSearchAccountFormIndividualsState; + + expect(finesSaPayloadService.transformPayload).toHaveBeenCalledWith( + expect.any(Object), + FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG, + ); + expect(transformedCriteria).not.toBe(finesSaStore.searchAccount().fsa_search_account_individuals_search_criteria); + expect(transformedCriteria.fsa_search_account_individuals_date_of_birth).toBe('1980-01-02'); + expect(finesSaStore.searchAccount().fsa_search_account_individuals_search_criteria).toEqual( + expect.objectContaining({ + fsa_search_account_individuals_date_of_birth: '02/01/1980', + }), + ); expect(opalFines.getDefendantAccounts).toHaveBeenCalledWith( expect.objectContaining({ active_accounts_only: true, @@ -182,6 +203,8 @@ describe('finesSaDefendantAccountsResolver (store-driven)', () => { }); it('defaults active_accounts_only to true when not set in store', async () => { + const transformPayloadSpy = vi.spyOn(finesSaPayloadService, 'transformPayload'); + // active flag omitted -> resolver should send true by default finesSaStore.setSearchAccount({ ...FINES_SA_SEARCH_ACCOUNT_STATE, @@ -198,6 +221,7 @@ describe('finesSaDefendantAccountsResolver (store-driven)', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any await lastValueFrom(execIndividuals(undefined as any, undefined as any) as Observable); + expect(transformPayloadSpy).toHaveBeenCalledWith(expect.any(Object), FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG); expect(opalFines.getDefendantAccounts).toHaveBeenCalledWith( expect.objectContaining({ active_accounts_only: true }), ); diff --git a/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.ts b/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.ts index 01b680a79d..e4c7a069f4 100644 --- a/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.ts +++ b/src/app/flows/fines/fines-sa/routing/resolvers/fines-sa-defendant-accounts/fines-sa-defendant-accounts.resolver.ts @@ -7,6 +7,8 @@ import { OPAL_FINES_DEFENDANT_ACCOUNT_SEARCH_PARAMS_DEFAULTS } from '@services/f import { OPAL_FINES_DEFENDANT_ACCOUNT_SEARCH_PARAMS_DEFENDANT_DEFAULTS } from '@services/fines/opal-fines-service/constants/opal-fines-defendant-account-search-params-defendant-defaults.constant'; import { OPAL_FINES_DEFENDANT_ACCOUNT_SEARCH_PARAMS_REFERENCE_DEFAULTS } from '@services/fines/opal-fines-service/constants/opal-fines-defendant-account-search-params-reference-defaults.constant'; import { of } from 'rxjs'; +import { FinesSaPayloadService } from '../../../services/fines-sa-payload.service'; +import { FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG } from '../../../services/constants/fines-sa-transform-items-config.constant'; /** * Resolver that retrieves defendant accounts based on current search criteria in the Fines SA flow. @@ -27,6 +29,7 @@ export const finesSaDefendantAccountsResolver = () => { const opalFinesService = inject(OpalFines); const finesSaStore = inject(FinesSaStore); + const finesSaPayloadService = inject(FinesSaPayloadService); const state = finesSaStore.searchAccount(); const activeTab = finesSaStore.activeTab(); @@ -82,20 +85,27 @@ export const finesSaDefendantAccountsResolver = // 3) Criteria based on searchType if (activeTab === 'individuals' && individualCriteria) { + const transformedIndividualCriteria = finesSaPayloadService.transformPayload( + { ...individualCriteria }, + FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG, + ); return opalFinesService.getDefendantAccounts({ ...baseSearchParams, reference_number: null, defendant: { ...OPAL_FINES_DEFENDANT_ACCOUNT_SEARCH_PARAMS_DEFENDANT_DEFAULTS, - surname: individualCriteria.fsa_search_account_individuals_last_name, - exact_match_surname: individualCriteria.fsa_search_account_individuals_last_name_exact_match ?? false, - forenames: individualCriteria.fsa_search_account_individuals_first_names, - exact_match_forenames: individualCriteria.fsa_search_account_individuals_first_names_exact_match ?? false, - birth_date: individualCriteria.fsa_search_account_individuals_date_of_birth, - national_insurance_number: individualCriteria.fsa_search_account_individuals_national_insurance_number, - address_line_1: individualCriteria.fsa_search_account_individuals_address_line_1, - postcode: individualCriteria.fsa_search_account_individuals_post_code, - include_aliases: individualCriteria.fsa_search_account_individuals_include_aliases ?? false, + surname: transformedIndividualCriteria.fsa_search_account_individuals_last_name, + exact_match_surname: + transformedIndividualCriteria.fsa_search_account_individuals_last_name_exact_match ?? false, + forenames: transformedIndividualCriteria.fsa_search_account_individuals_first_names, + exact_match_forenames: + transformedIndividualCriteria.fsa_search_account_individuals_first_names_exact_match ?? false, + birth_date: transformedIndividualCriteria.fsa_search_account_individuals_date_of_birth, + national_insurance_number: + transformedIndividualCriteria.fsa_search_account_individuals_national_insurance_number, + address_line_1: transformedIndividualCriteria.fsa_search_account_individuals_address_line_1, + postcode: transformedIndividualCriteria.fsa_search_account_individuals_post_code, + include_aliases: transformedIndividualCriteria.fsa_search_account_individuals_include_aliases ?? false, organisation: isCompany, }, active_accounts_only: state.fsa_search_account_active_accounts_only ?? true, diff --git a/src/app/flows/fines/fines-sa/services/constants/fines-sa-transform-items-config.constant.ts b/src/app/flows/fines/fines-sa/services/constants/fines-sa-transform-items-config.constant.ts new file mode 100644 index 0000000000..ff90233390 --- /dev/null +++ b/src/app/flows/fines/fines-sa/services/constants/fines-sa-transform-items-config.constant.ts @@ -0,0 +1,15 @@ +import { TRANSFORM_ITEM_DEFAULTS } from '@hmcts/opal-frontend-common/services/transformation-service/constants'; +import { ITransformItem } from '@hmcts/opal-frontend-common/services/transformation-service/interfaces'; + +const FINES_SA_BUILD_PAYLOAD_DATE_FORMAT = { + ...TRANSFORM_ITEM_DEFAULTS, + transformType: 'date', + dateConfig: { + inputFormat: 'dd/MM/yyyy', + outputFormat: 'yyyy-MM-dd', + }, +}; + +export const FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG: ITransformItem[] = [ + { key: 'fsa_search_account_individuals_date_of_birth', ...FINES_SA_BUILD_PAYLOAD_DATE_FORMAT }, +]; diff --git a/src/app/flows/fines/fines-sa/services/fines-sa-payload.service.spec.ts b/src/app/flows/fines/fines-sa/services/fines-sa-payload.service.spec.ts new file mode 100644 index 0000000000..01d3193edb --- /dev/null +++ b/src/app/flows/fines/fines-sa/services/fines-sa-payload.service.spec.ts @@ -0,0 +1,39 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { FinesSaPayloadService } from './fines-sa-payload.service'; +import { TestBed } from '@angular/core/testing'; +import { FINES_SA_SEARCH_ACCOUNT_FORM_INDIVIDUALS_STATE_MOCK } from '../fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-individuals/mocks/fines-sa-search-account-form-individuals-state.mock'; +import { FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG } from './constants/fines-sa-transform-items-config.constant'; +import { DateService } from '@hmcts/opal-frontend-common/services/date-service'; + +describe('FinesSaPayloadService', () => { + let service: FinesSaPayloadService; + let dateService: DateService; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [FinesSaPayloadService], + }); + service = TestBed.inject(FinesSaPayloadService); + dateService = TestBed.inject(DateService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + it('should transform payload using the transformation service', () => { + const dateOfBirth = + FINES_SA_SEARCH_ACCOUNT_FORM_INDIVIDUALS_STATE_MOCK.fsa_search_account_individuals_date_of_birth; + + if (dateOfBirth === null) { + throw new Error('Expected date of birth in individuals state mock'); + } + + const transformedDateOfBirth = dateService.getFromFormatToFormat(dateOfBirth, 'dd/MM/yyyy', 'yyyy-MM-dd'); + const result = service.transformPayload( + FINES_SA_SEARCH_ACCOUNT_FORM_INDIVIDUALS_STATE_MOCK, + FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG, + ); + expect(result.fsa_search_account_individuals_date_of_birth).toBe(transformedDateOfBirth); + }); +}); diff --git a/src/app/flows/fines/fines-sa/services/fines-sa-payload.service.ts b/src/app/flows/fines/fines-sa/services/fines-sa-payload.service.ts new file mode 100644 index 0000000000..6b0e5168b7 --- /dev/null +++ b/src/app/flows/fines/fines-sa/services/fines-sa-payload.service.ts @@ -0,0 +1,27 @@ +import { inject, Injectable } from '@angular/core'; +import { TransformationService } from '@hmcts/opal-frontend-common/services/transformation-service'; + +import { ITransformItem } from '@hmcts/opal-frontend-common/services/transformation-service/interfaces'; + +@Injectable({ + providedIn: 'root', +}) +export class FinesSaPayloadService { + private readonly transformationService = inject(TransformationService); + + /** + + * Transforms the given finesSaPayload object by applying the transformations + * defined in the FINES_SA_BUILD_TRANSFORM_ITEMS_CONFIG. + * + * @param finesSaPayload - The payload object to be transformed. + * @returns The transformed payload object. + */ + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ + public transformPayload( + finesSaPayload: T, + transformItemsConfig: ITransformItem[], + ): T { + return this.transformationService.transformObjectValues(finesSaPayload, transformItemsConfig); + } +} diff --git a/src/test-setup.ts b/src/test-setup.ts index eaab8306b3..393b37b235 100644 --- a/src/test-setup.ts +++ b/src/test-setup.ts @@ -1,7 +1,17 @@ +import '@angular/compiler'; +import { getTestBed } from '@angular/core/testing'; +import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; +import 'zone.js'; import 'zone.js/testing'; import { GovukRadioComponent } from '@hmcts/opal-frontend-common/components/govuk/govuk-radio'; import { MojDatePickerComponent } from '@hmcts/opal-frontend-common/components/moj/moj-date-picker'; +const testBed = getTestBed(); + +if (!testBed.platform) { + testBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting()); +} + // JSDOM cannot initialize GOV.UK and MOJ frontend widgets used by shared components. // Keep component behavior testable without executing browser-only initializers. Object.defineProperty(MojDatePickerComponent.prototype, 'configureDatePicker', { diff --git a/vitest.config.ts b/vitest.config.ts index e31177d2c6..c67bab7426 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -4,6 +4,12 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ resolve: { alias: { + '@app': resolve(__dirname, 'src/app'), + '@constants': resolve(__dirname, 'src/app/constants'), + '@services/fines': resolve(__dirname, 'src/app/flows/fines/services'), + '@routing/pages': resolve(__dirname, 'src/app/pages/routing'), + '@routing/fines': resolve(__dirname, 'src/app/flows/fines/routing'), + '@routing/flows': resolve(__dirname, 'src/app/flows/routing'), '@ministryofjustice/frontend/moj/all': resolve(__dirname, 'node_modules/@ministryofjustice/frontend/moj/all.mjs'), }, }, From 65dda7f9ddd37e38b315caee532cc57b96f190cd Mon Sep 17 00:00:00 2001 From: Frankie Moran Date: Tue, 31 Mar 2026 14:51:13 +0100 Subject: [PATCH 2/5] Get the extension working successfully. Add new codex skill to help --- .../opal-frontend/opal-vitest-guard/SKILL.md | 228 ++++++++++++++++++ .../references/unit-test-checklist.md | 31 +++ .vscode/settings.json | 1 + src/app/app.component.spec.ts | 5 +- ...ines-acc-banner-messages.component.spec.ts | 2 +- ...fines-acc-summary-header.component.spec.ts | 5 +- ...search-account-form-companies.validator.ts | 4 +- ...arch-account-form-individuals.validator.ts | 4 +- ...ils-review-offence-imposition.component.ts | 2 +- ...-review-account-payment-terms.component.ts | 2 +- ...-payload-add-account-fixed-penalty.mock.ts | 2 +- .../fines-mac-payload-add-account.mock.ts | 2 +- ...es-mac-payload-build-account-base.utils.ts | 2 +- ...yload-build-account-offences.utils.spec.ts | 2 +- ...ac-payload-build-account-offences.utils.ts | 2 +- ...yload-build-account-payment-terms.utils.ts | 2 +- ...ad-map-account-payment-terms.utils.spec.ts | 2 +- ...payload-map-account-payment-terms.utils.ts | 2 +- ...ac-payload-account-account-initial.mock.ts | 2 +- ...-mac-payload-account-details-state.mock.ts | 2 +- ...ayload-fixed-penalty-details-state.mock.ts | 2 +- ...search-account-form-companies.validator.ts | 4 +- ...arch-account-form-individuals.validator.ts | 4 +- ...-account-form-minor-creditors.component.ts | 2 +- src/test-setup.vscode.ts | 111 +++++++++ vitest.config.ts | 2 + vitest.vscode.config.ts | 12 + 27 files changed, 411 insertions(+), 30 deletions(-) create mode 100644 .codex/skills/opal-frontend/opal-vitest-guard/SKILL.md create mode 100644 .codex/skills/opal-frontend/opal-vitest-guard/references/unit-test-checklist.md create mode 100644 src/test-setup.vscode.ts create mode 100644 vitest.vscode.config.ts diff --git a/.codex/skills/opal-frontend/opal-vitest-guard/SKILL.md b/.codex/skills/opal-frontend/opal-vitest-guard/SKILL.md new file mode 100644 index 0000000000..065796d3d3 --- /dev/null +++ b/.codex/skills/opal-frontend/opal-vitest-guard/SKILL.md @@ -0,0 +1,228 @@ +--- +name: opal-vitest-guard +description: Review, write, or fix unit tests in TypeScript repos that use Vitest, Angular TestBed, and component-driven rendering. Use this when adding tests, debugging flaky or failing tests, preventing ExpressionChangedAfterItHasBeenCheckedError, improving beforeEach setup, or standardizing safe render patterns. Do not use for end-to-end tests, backend integration tests, or broad refactors unrelated to test behavior. +--- + +# OPAL Vitest Guard + +Use this skill when working on unit tests in frontend TypeScript repositories, especially Angular + Vitest setups. Favor small, deterministic fixes that preserve product behavior while making tests reliable. + +## Goals + +1. Prevent common unit-test regressions before they are introduced. +2. Diagnose failures by separating test-harness problems from real product bugs. +3. Standardize safe render/setup patterns for Angular component tests. +4. Keep tests readable, local, and minimally coupled to framework timing. + +## Default stance + +- Treat the test runner as correct unless there is strong evidence otherwise. +- Prefer fixing the test setup order before changing component production code. +- Prefer one explicit render point per test. +- Prefer state arrangement before the first render. +- Prefer the smallest change that makes intent clearer. +- Do not mask real bugs with excessive `tick()`, duplicate `detectChanges()`, or broad async wrappers. + +## Triage workflow + +Follow this order every time: + +### 1) Classify the failure + +Put the failure into one of these buckets: + +- **Suite load / module resolution** + - Examples: cannot resolve import, transform failed, missing file, bad alias. + - Action: fix pathing, alias config, or moved file references before touching assertions. + +- **Framework lifecycle / render timing** + - Examples: `ExpressionChangedAfterItHasBeenCheckedError`, DOM not updated yet, child component missing because nothing rendered. + - Action: inspect render order, `beforeEach`, input assignment timing, selectors/signals/spies, and async stabilization. + +- **Real assertion / product logic failure** + - Examples: wrong text, wrong emitted event, wrong state transition. + - Action: verify expected behavior, then fix either the test expectation or the component logic. + +### 2) Inspect shared setup first + +Look at `beforeEach` before editing individual tests. + +Red flags: + +- `fixture.detectChanges()` in shared setup +- mutating `@Input()`-like properties after shared render +- spies returning one value in `beforeEach` and a different value inside a test after render +- store/signal values updated after component creation +- timer setup in `ngOnInit()` combined with first render in the same test + +### 3) Render once, after arrangement + +Default pattern: + +```ts +beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [MyComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(MyComponent); + component = fixture.componentInstance; + + // Set defaults only. No detectChanges here. + component.someInput = false; + vi.spyOn(store, 'flag').mockReturnValue(false); +}); + +it('renders state X', () => { + component.someInput = true; + vi.spyOn(store, 'flag').mockReturnValue(true); + + fixture.detectChanges(); + + expect(...).toBe(...); +}); +``` + +Only deviate when the test is explicitly about multiple renders or reactive updates over time. + +## Angular-specific guardrails + +### Prevent `ExpressionChangedAfterItHasBeenCheckedError` + +When this error appears, check for a value that changes between the initial check and the next check. + +Common causes: + +- shared `fixture.detectChanges()` in `beforeEach` +- flipping a template-bound field after initial render +- changing spy returns after the component already read them +- store/signal selectors emitting after the component was created when the test intended initial state +- invoking lifecycle methods manually in addition to `detectChanges()` + +Preferred fixes: + +1. Remove shared `fixture.detectChanges()` from `beforeEach`. +2. Arrange final test state before first render. +3. Create the fixture after mocks are ready if the component reads dependencies during construction/init. +4. Use a single, explicit async strategy per test. +5. If the test only cares about rendered state, assert state directly instead of recreating the full timing path. + +### Safe patterns + +**Inputs first, render second** + +```ts +component.hasPermission = true; +fixture.detectChanges(); +``` + +**Spy first, render second** + +```ts +vi.spyOn(component.accountStore, 'successMessage').mockReturnValue('Saved'); +fixture.detectChanges(); +``` + +**Render child before querying it** + +```ts +fixture.detectChanges(); +const banner = fixture.debugElement.query(By.directive(BannerComponent)); +``` + +### Avoid these patterns + +```ts +fixture.detectChanges(); +component.hasPermission = true; +fixture.detectChanges(); +``` + +```ts +fixture.detectChanges(); +vi.spyOn(store, 'flag').mockReturnValue(true); +fixture.detectChanges(); +``` + +```ts +component.ngOnInit(); +fixture.detectChanges(); +``` + +Only call lifecycle hooks manually when the test specifically targets that hook and you are not also relying on the normal render path in a conflicting way. + +## Async and timers + +Use only one timing model per test where possible: + +- `await fixture.whenStable()` for promise-driven stabilization +- `fakeAsync` + `tick()` for Angular fake timers +- `vi.useFakeTimers()` + timer advancement for Vitest timer-driven code + +Do not stack multiple timing models unless necessary and understood. + +Checklist: + +- Did the component schedule work in `ngOnInit`, `ngAfterViewInit`, or an effect? +- Did the test render before the mocked async value was ready? +- Is the test asserting DOM before stabilization? +- Can the test assert state or emitted output instead of a full timer loop? + +## Module-resolution guardrails + +When tests fail to load: + +1. Verify the imported file still exists. +2. Verify relative path depth from the importing file. +3. Check whether the repo relies on TS path aliases. +4. Ensure Vitest/Vite resolves the same aliases as TypeScript. +5. Fix the import/config issue before editing test assertions. + +Never diagnose a suite-load error as a UI problem in the test explorer until the terminal runner is green. + +## Review checklist for new or changed tests + +Before finalizing a patch, check all of these: + +- The test passes from the terminal runner. +- Shared setup does not render unless every test needs the exact same initial DOM. +- Template-bound values are not flipped after first render unless intentionally testing reactivity. +- Spies/selectors/mocks are in their final state before render. +- DOM queries happen after render. +- Async strategy is consistent and minimal. +- Console noise is not mistaken for a failing assertion. +- The test name describes behavior, not implementation details. +- Assertions are specific enough to catch regressions. +- Production code was not weakened just to satisfy the test. + +## Output expectations + +When using this skill, produce: + +1. A short diagnosis of the failure bucket. +2. The smallest safe patch. +3. A brief explanation of why the original pattern failed. +4. A prevention note the team can reuse. + +## Preferred remediation language + +Use wording like: + +- "Render after arranging final state." +- "Remove shared detectChanges from beforeEach." +- "This is a real assertion failure, not a Vitest Explorer failure." +- "The suite is failing to load, so fix module resolution first." +- "This test only needs the final rendered state; it does not need the full init/timer lifecycle." + +## When not to apply this skill + +Do not use this skill for: + +- Cypress/Playwright end-to-end tests +- API integration tests +- broad test rewrites without a concrete failure or standardization goal +- changes whose main purpose is snapshot churn or stylistic renaming + +## Reference files + +See `references/unit-test-checklist.md` for a compact checklist teams can keep nearby. diff --git a/.codex/skills/opal-frontend/opal-vitest-guard/references/unit-test-checklist.md b/.codex/skills/opal-frontend/opal-vitest-guard/references/unit-test-checklist.md new file mode 100644 index 0000000000..4b6b9c1c00 --- /dev/null +++ b/.codex/skills/opal-frontend/opal-vitest-guard/references/unit-test-checklist.md @@ -0,0 +1,31 @@ +# Unit Test Checklist + +## Before writing the assertion + +- Arrange final inputs, spies, selectors, and mock store values first. +- Do not call `fixture.detectChanges()` in shared setup unless every test truly needs the same rendered DOM. +- Decide whether the test is synchronous, promise-based, or timer-based. + +## When a test fails + +- If the suite does not load, fix imports or config first. +- If Angular throws `ExpressionChangedAfterItHasBeenCheckedError`, move state setup before the first render. +- If a DOM query fails, verify the component was rendered before querying. +- If logs are noisy but all tests pass, treat the logs separately from pass/fail status. + +## Safe default + +```ts +beforeEach(async () => { + await TestBed.configureTestingModule({ imports: [MyComponent] }).compileComponents(); + fixture = TestBed.createComponent(MyComponent); + component = fixture.componentInstance; + // arrange defaults only +}); + +it('renders expected state', () => { + // arrange final state + fixture.detectChanges(); + // assert +}); +``` diff --git a/.vscode/settings.json b/.vscode/settings.json index 7e772ec256..a19a5b8b3a 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -19,6 +19,7 @@ "typescript.tsserver.experimental.enableProjectDiagnostics": true, "javascript.tsserver.experimental.enableProjectDiagnostics": true, + "vitest.rootConfig": "vitest.vscode.config.ts", "sonarlint.connectedMode.project": { "connectionId": "HMCTS SonarQube", diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index d029288e4f..8923c59c97 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -149,11 +149,8 @@ describe('AppComponent - browser', () => { dateService.convertMillisecondsToMinutes.mockReturnValue(5); dateService.calculateMinutesDifference.mockReturnValue(0); - component.ngOnInit(); - component['initializeTimeoutInterval'](); + component.showExpiredWarning = true; - // Simulate timer tick - vi.advanceTimersByTime(component['POLL_INTERVAL'] * 1000); fixture.detectChanges(); expect(component.showExpiredWarning).toBe(true); diff --git a/src/app/flows/fines/fines-acc/fines-acc-banner-messages/fines-acc-banner-messages.component.spec.ts b/src/app/flows/fines/fines-acc/fines-acc-banner-messages/fines-acc-banner-messages.component.spec.ts index 475dce3cd9..aff437fda0 100644 --- a/src/app/flows/fines/fines-acc/fines-acc-banner-messages/fines-acc-banner-messages.component.spec.ts +++ b/src/app/flows/fines/fines-acc/fines-acc-banner-messages/fines-acc-banner-messages.component.spec.ts @@ -16,10 +16,10 @@ describe('FinesAccBannerMessagesComponent', () => { component = fixture.componentInstance; component.hasVersionMismatch = false; component.successMessage = null; - fixture.detectChanges(); }); it('should create', () => { + fixture.detectChanges(); expect(component).toBeTruthy(); }); diff --git a/src/app/flows/fines/fines-acc/fines-acc-summary-header/fines-acc-summary-header.component.spec.ts b/src/app/flows/fines/fines-acc/fines-acc-summary-header/fines-acc-summary-header.component.spec.ts index 178566c663..3ed1900b19 100644 --- a/src/app/flows/fines/fines-acc/fines-acc-summary-header/fines-acc-summary-header.component.spec.ts +++ b/src/app/flows/fines/fines-acc/fines-acc-summary-header/fines-acc-summary-header.component.spec.ts @@ -25,8 +25,6 @@ describe('FinesAccSummaryHeaderComponent', () => { } as unknown as typeof component.accountStore; component.hasAddAccountActivityNotePermission = false; - - fixture.detectChanges(); }); it('should create', () => { @@ -60,8 +58,9 @@ describe('FinesAccSummaryHeaderComponent', () => { }); it('should clear success message when banner emits clearSuccessMessage', () => { - const banner = fixture.debugElement.query(By.directive(FinesAccBannerMessagesComponent)); + fixture.detectChanges(); + const banner = fixture.debugElement.query(By.directive(FinesAccBannerMessagesComponent)); banner.triggerEventHandler('clearSuccessMessage'); expect(component.accountStore.clearSuccessMessage).toHaveBeenCalled(); diff --git a/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-companies/validators/fines-con-search-account-form-companies.validator.ts b/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-companies/validators/fines-con-search-account-form-companies.validator.ts index b523038eac..dd22c7aca2 100644 --- a/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-companies/validators/fines-con-search-account-form-companies.validator.ts +++ b/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-companies/validators/fines-con-search-account-form-companies.validator.ts @@ -1,6 +1,6 @@ import { AbstractControl, ValidationErrors, ValidatorFn } from '@angular/forms'; -import { createConditionalRequiredValidator } from 'src/app/flows/fines/validators/conditional-required.validator'; -import { ConditionalRequiredRuleConfig } from 'src/app/flows/fines/validators/interfaces/conditional-required-rule-config.interface'; +import { createConditionalRequiredValidator } from '../../../../../../validators/conditional-required.validator'; +import { ConditionalRequiredRuleConfig } from '../../../../../../validators/interfaces/conditional-required-rule-config.interface'; /** * Cross-field validator for CON Companies search criteria conditional required rules. diff --git a/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-individuals/validators/fines-con-search-account-form-individuals.validator.ts b/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-individuals/validators/fines-con-search-account-form-individuals.validator.ts index b870404a53..b53add1358 100644 --- a/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-individuals/validators/fines-con-search-account-form-individuals.validator.ts +++ b/src/app/flows/fines/fines-con/consolidate-acc/fines-con-search-account/fines-con-search-account-form/fines-con-search-account-form-individuals/validators/fines-con-search-account-form-individuals.validator.ts @@ -1,6 +1,6 @@ import { AbstractControl, ValidationErrors, ValidatorFn } from '@angular/forms'; -import { createConditionalRequiredValidator } from 'src/app/flows/fines/validators/conditional-required.validator'; -import { ConditionalRequiredRuleConfig } from 'src/app/flows/fines/validators/interfaces/conditional-required-rule-config.interface'; +import { createConditionalRequiredValidator } from '../../../../../../validators/conditional-required.validator'; +import { ConditionalRequiredRuleConfig } from '../../../../../../validators/interfaces/conditional-required-rule-config.interface'; /** * Cross-field validator for Individuals search criteria conditional required rules. diff --git a/src/app/flows/fines/fines-mac/fines-mac-offence-details/fines-mac-offence-details-review-offence/fines-mac-offence-details-review-offence-imposition/fines-mac-offence-details-review-offence-imposition.component.ts b/src/app/flows/fines/fines-mac/fines-mac-offence-details/fines-mac-offence-details-review-offence/fines-mac-offence-details-review-offence-imposition/fines-mac-offence-details-review-offence-imposition.component.ts index 86330c559d..740b6c3e9e 100644 --- a/src/app/flows/fines/fines-mac/fines-mac-offence-details/fines-mac-offence-details-review-offence/fines-mac-offence-details-review-offence-imposition/fines-mac-offence-details-review-offence-imposition.component.ts +++ b/src/app/flows/fines/fines-mac/fines-mac-offence-details/fines-mac-offence-details-review-offence/fines-mac-offence-details-review-offence-imposition/fines-mac-offence-details-review-offence-imposition.component.ts @@ -20,7 +20,7 @@ import { CommonModule } from '@angular/common'; import { FINES_MAC_OFFENCE_DETAILS_REVIEW_OFFENCE_IMPOSITION_DEFAULT_VALUES } from './constants/fines-mac-offence-details-review-offence-imposition-default-values.constant'; import { FinesMacStore } from '../../../stores/fines-mac.store'; import { UtilsService } from '@hmcts/opal-frontend-common/services/utils-service'; -import { FinesNotProvidedComponent } from 'src/app/flows/fines/components/fines-not-provided/fines-not-provided.component'; +import { FinesNotProvidedComponent } from '../../../../components/fines-not-provided/fines-not-provided.component'; @Component({ selector: 'app-fines-mac-offence-details-review-offence-imposition', diff --git a/src/app/flows/fines/fines-mac/fines-mac-review-account/fines-mac-review-account-payment-terms/fines-mac-review-account-payment-terms.component.ts b/src/app/flows/fines/fines-mac/fines-mac-review-account/fines-mac-review-account-payment-terms/fines-mac-review-account-payment-terms.component.ts index eb9824864c..cf1505fa2a 100644 --- a/src/app/flows/fines/fines-mac/fines-mac-review-account/fines-mac-review-account-payment-terms/fines-mac-review-account-payment-terms.component.ts +++ b/src/app/flows/fines/fines-mac/fines-mac-review-account/fines-mac-review-account-payment-terms/fines-mac-review-account-payment-terms.component.ts @@ -12,7 +12,7 @@ import { FINES_MAC_PAYMENT_TERMS_ENFORCEMENT_ACTION_OPTIONS } from '../../fines- import { IFinesMacPaymentTermsEnforcementActionsOptions } from '../../fines-mac-payment-terms/interfaces/fines-mac-payment-terms-enforcement-actions-options.interface'; import { IFinesMacPaymentTermsPermissions } from '../../fines-mac-payment-terms/interfaces/fines-mac-payment-terms-permissions.interface'; import { FINES_PAYMENT_TERMS_FREQUENCY_OPTIONS } from '../../../constants/fines-payment-terms-frequency-options.constant'; -import { IOpalFinesBusinessUnit } from 'src/app/flows/fines/services/opal-fines-service/interfaces/opal-fines-business-unit.interface'; +import { IOpalFinesBusinessUnit } from '../../../services/opal-fines-service/interfaces/opal-fines-business-unit.interface'; import { FINES_PERMISSIONS } from '@constants/fines-permissions.constant'; import { FinesMacReviewAccountChangeLinkComponent } from '../fines-mac-review-account-change-link/fines-mac-review-account-change-link.component'; import { DateService } from '@hmcts/opal-frontend-common/services/date-service'; diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account-fixed-penalty.mock.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account-fixed-penalty.mock.ts index 28109ca269..9898b8278b 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account-fixed-penalty.mock.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account-fixed-penalty.mock.ts @@ -2,7 +2,7 @@ import { OPAL_USER_STATE_MOCK } from '@hmcts/opal-frontend-common/services/opal- import { FINES_MAC_PAYLOAD_ACCOUNT_DEFENDANT } from '../constants/fines-mac-payload-account-defendant.constant'; import { IFinesMacAddAccountPayload } from '../interfaces/fines-mac-payload-add-account.interfaces'; import { FINES_MAC_PAYLOAD_ACCOUNT_DEFENDANT_INDIVIDUAL_MOCK } from '../utils/mocks/fines-mac-payload-account-defendant-individual.mock'; -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../constants/fines-account-types.constant'; export const FINES_MAC_PAYLOAD_ADD_ACCOUNT_FIXED_PENALTY_MOCK: IFinesMacAddAccountPayload = { draft_account_id: null, diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account.mock.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account.mock.ts index 59b4e5eb28..82623341f3 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account.mock.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/mocks/fines-mac-payload-add-account.mock.ts @@ -3,7 +3,7 @@ import { FINES_MAC_PAYLOAD_ACCOUNT_DEFENDANT_PARENT_GUARDIAN } from '../constant import { FINES_MAC_PAYLOAD_ACCOUNT_DEFENDANT } from '../constants/fines-mac-payload-account-defendant.constant'; import { IFinesMacAddAccountPayload } from '../interfaces/fines-mac-payload-add-account.interfaces'; import { FINES_MAC_PAYLOAD_ACCOUNT_DEFENDANT_INDIVIDUAL_MOCK } from '../utils/mocks/fines-mac-payload-account-defendant-individual.mock'; -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../constants/fines-account-types.constant'; export const FINES_MAC_PAYLOAD_ADD_ACCOUNT: IFinesMacAddAccountPayload = { draft_account_id: null, diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-base.utils.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-base.utils.ts index 3fdc1c33c0..1277a7b400 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-base.utils.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-base.utils.ts @@ -1,4 +1,4 @@ -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../../constants/fines-account-types.constant'; import { IFinesMacAccountDetailsState } from '../../../../fines-mac-account-details/interfaces/fines-mac-account-details-state.interface'; import { IFinesMacCourtDetailsState } from '../../../../fines-mac-court-details/interfaces/fines-mac-court-details-state.interface'; import { IFinesMacFixedPenaltyDetailsStoreState } from '../../../../fines-mac-fixed-penalty-details/interfaces/fines-mac-fixed-penalty-details-store-state.interface'; diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.spec.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.spec.ts index 4cc82df098..7d9bab3af4 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.spec.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.spec.ts @@ -9,7 +9,7 @@ import { FINES_MAC_PAYLOAD_OFFENCE_DETAILS_MINOR_CREDITOR_STATE } from '../mocks import { IFinesMacCourtDetailsState } from '../../../../fines-mac-court-details/interfaces/fines-mac-court-details-state.interface'; import { IFinesMacFixedPenaltyDetailsStoreState } from '../../../../fines-mac-fixed-penalty-details/interfaces/fines-mac-fixed-penalty-details-store-state.interface'; import { FINES_MAC_PAYLOAD_FIXED_PENALTY_DETAILS_STATE_MOCK } from '../mocks/state/fines-mac-payload-fixed-penalty-details-state.mock'; -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../../constants/fines-account-types.constant'; import { afterAll, beforeEach, describe, expect, it } from 'vitest'; describe('finesMacPayloadBuildAccountOffences', () => { diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.ts index 1877a8e09d..0a475b1365 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-offences.utils.ts @@ -1,4 +1,4 @@ -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../../constants/fines-account-types.constant'; import { IFinesMacFixedPenaltyDetailsStoreState } from '../../../../fines-mac-fixed-penalty-details/interfaces/fines-mac-fixed-penalty-details-store-state.interface'; import { IFinesMacOffenceDetailsMinorCreditorForm } from '../../../../fines-mac-offence-details/fines-mac-offence-details-minor-creditor/interfaces/fines-mac-offence-details-minor-creditor-form.interface'; import { IFinesMacOffenceDetailsForm } from '../../../../fines-mac-offence-details/interfaces/fines-mac-offence-details-form.interface'; diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-payment-terms.utils.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-payment-terms.utils.ts index c84c6cfa97..984425f197 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-payment-terms.utils.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-build-account/fines-mac-payload-build-account-payment-terms.utils.ts @@ -2,7 +2,7 @@ import { IFinesMacPaymentTermsState } from '../../../../fines-mac-payment-terms/ import { IFinesMacPayloadAccountPaymentTermsEnforcement } from '../interfaces/fines-mac-payload-account-payment-terms-enforcement.interface'; import { IFinesMacPayloadAccountPaymentTermsEnforcementResultResponse } from '../interfaces/fines-mac-payload-account-payment-terms-enforcement-result-response.interface'; import { IFinesMacPayloadAccountPaymentTerms } from '../interfaces/fines-mac-payload-account-payment-terms.interface'; -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../../constants/fines-account-types.constant'; /** * Builds an enforcement response object for fines MAC payment terms. diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.spec.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.spec.ts index 369894ac16..0d1a257cee 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.spec.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.spec.ts @@ -1,7 +1,7 @@ import { finesMacPayloadMapAccountPaymentTerms } from './fines-mac-payload-map-account-payment-terms.utils'; import { IFinesMacState } from '../../../../interfaces/fines-mac-state.interface'; import { IFinesMacPayloadAccount } from '../../interfaces/fines-mac-payload-account.interface'; -import { FINES_PAYMENT_TERMS_OPTIONS } from 'src/app/flows/fines/constants/fines-payment-terms-options.constant'; +import { FINES_PAYMENT_TERMS_OPTIONS } from '../../../../../constants/fines-payment-terms-options.constant'; import { describe, beforeEach, afterAll, it, expect } from 'vitest'; describe('finesMacPayloadMapAccountPaymentTerms', () => { diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.ts index 4c7e3646f4..f2212acc57 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/fines-mac-payload-map-account/fines-mac-payload-map-account-payment-terms.utils.ts @@ -1,4 +1,4 @@ -import { FINES_PAYMENT_TERMS_OPTIONS } from 'src/app/flows/fines/constants/fines-payment-terms-options.constant'; +import { FINES_PAYMENT_TERMS_OPTIONS } from '../../../../../constants/fines-payment-terms-options.constant'; import { IFinesMacPaymentTermsState } from '../../../../fines-mac-payment-terms/interfaces/fines-mac-payment-terms-state.interface'; import { IFinesMacState } from '../../../../interfaces/fines-mac-state.interface'; import { IFinesMacPayloadAccount } from '../../interfaces/fines-mac-payload-account.interface'; diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/fines-mac-payload-account-account-initial.mock.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/fines-mac-payload-account-account-initial.mock.ts index 87e60997d9..7bd54caecc 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/fines-mac-payload-account-account-initial.mock.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/fines-mac-payload-account-account-initial.mock.ts @@ -1,4 +1,4 @@ -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../../constants/fines-account-types.constant'; import { IFinesMacPayloadAccountAccountInitial } from '../../interfaces/fines-mac-payload-account-initial.interface'; export const FINES_MAC_PAYLOAD_ACCOUNT_ACCOUNT_INITIAL_MOCK: IFinesMacPayloadAccountAccountInitial = { diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-account-details-state.mock.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-account-details-state.mock.ts index d80c196bda..db17300fa4 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-account-details-state.mock.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-account-details-state.mock.ts @@ -1,5 +1,5 @@ import { IFinesMacAccountDetailsState } from '../../../../../fines-mac-account-details/interfaces/fines-mac-account-details-state.interface'; -import { FINES_ACCOUNT_TYPES } from 'src/app/flows/fines/constants/fines-account-types.constant'; +import { FINES_ACCOUNT_TYPES } from '../../../../../../constants/fines-account-types.constant'; export const FINES_MAC_PAYLOAD_ACCOUNT_DETAILS_STATE_MOCK: IFinesMacAccountDetailsState = { fm_create_account_account_type: FINES_ACCOUNT_TYPES['Conditional Caution'], diff --git a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-fixed-penalty-details-state.mock.ts b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-fixed-penalty-details-state.mock.ts index 24c425c780..c2facb7983 100644 --- a/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-fixed-penalty-details-state.mock.ts +++ b/src/app/flows/fines/fines-mac/services/fines-mac-payload/utils/mocks/state/fines-mac-payload-fixed-penalty-details-state.mock.ts @@ -1,4 +1,4 @@ -import { IFinesMacFixedPenaltyDetailsStoreState } from 'src/app/flows/fines/fines-mac/fines-mac-fixed-penalty-details/interfaces/fines-mac-fixed-penalty-details-store-state.interface'; +import { IFinesMacFixedPenaltyDetailsStoreState } from '../../../../../../fines-mac/fines-mac-fixed-penalty-details/interfaces/fines-mac-fixed-penalty-details-store-state.interface'; export const FINES_MAC_PAYLOAD_FIXED_PENALTY_DETAILS_STATE_MOCK: IFinesMacFixedPenaltyDetailsStoreState = { fm_offence_details_notice_number: '12345', diff --git a/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-companies/validators/fines-sa-search-account-form-companies.validator.ts b/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-companies/validators/fines-sa-search-account-form-companies.validator.ts index 4de6b94d85..69b90e72f8 100644 --- a/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-companies/validators/fines-sa-search-account-form-companies.validator.ts +++ b/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-companies/validators/fines-sa-search-account-form-companies.validator.ts @@ -1,6 +1,6 @@ import { AbstractControl, ValidationErrors, ValidatorFn } from '@angular/forms'; -import { createConditionalRequiredValidator } from 'src/app/flows/fines/validators/conditional-required.validator'; -import { ConditionalRequiredRuleConfig } from 'src/app/flows/fines/validators/interfaces/conditional-required-rule-config.interface'; +import { createConditionalRequiredValidator } from '../../../../../../validators/conditional-required.validator'; +import { ConditionalRequiredRuleConfig } from '../../../../../../validators/interfaces/conditional-required-rule-config.interface'; /** * Cross-field validator for SA Companies search criteria conditional required rules. diff --git a/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-individuals/validators/fines-sa-search-account-form-individuals.validator.ts b/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-individuals/validators/fines-sa-search-account-form-individuals.validator.ts index 25434d5f1d..64af408e48 100644 --- a/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-individuals/validators/fines-sa-search-account-form-individuals.validator.ts +++ b/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-individuals/validators/fines-sa-search-account-form-individuals.validator.ts @@ -1,6 +1,6 @@ import { AbstractControl, ValidationErrors, ValidatorFn } from '@angular/forms'; -import { createConditionalRequiredValidator } from 'src/app/flows/fines/validators/conditional-required.validator'; -import { ConditionalRequiredRuleConfig } from 'src/app/flows/fines/validators/interfaces/conditional-required-rule-config.interface'; +import { createConditionalRequiredValidator } from '../../../../../../validators/conditional-required.validator'; +import { ConditionalRequiredRuleConfig } from '../../../../../../validators/interfaces/conditional-required-rule-config.interface'; /** * Cross-field validator for SA Individuals search criteria conditional required rules. diff --git a/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-minor-creditors/fines-sa-search-account-form-minor-creditors.component.ts b/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-minor-creditors/fines-sa-search-account-form-minor-creditors.component.ts index cf303ecc16..a992ed8a00 100644 --- a/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-minor-creditors/fines-sa-search-account-form-minor-creditors.component.ts +++ b/src/app/flows/fines/fines-sa/fines-sa-search/fines-sa-search-account/fines-sa-search-account-form/fines-sa-search-account-form-minor-creditors/fines-sa-search-account-form-minor-creditors.component.ts @@ -11,7 +11,7 @@ import { GovukRadiosConditionalComponent, } from '@hmcts/opal-frontend-common/components/govuk/govuk-radio'; import { GovukTextInputComponent } from '@hmcts/opal-frontend-common/components/govuk/govuk-text-input'; -import { FINES_MINOR_CREDITOR_TYPES } from 'src/app/flows/fines/constants/fines-minor-creditor-types.constant'; +import { FINES_MINOR_CREDITOR_TYPES } from '../../../../../constants/fines-minor-creditor-types.constant'; import { IGovUkRadioOptions } from '@hmcts/opal-frontend-common/components/govuk/govuk-radio/interfaces'; import { AbstractNestedFormBaseComponent } from '@hmcts/opal-frontend-common/components/abstract/abstract-nested-form-base'; import { requiredMinorCreditorDataValidator } from './validators/fines-sa-search-account-form-minor-creditors.validator'; diff --git a/src/test-setup.vscode.ts b/src/test-setup.vscode.ts new file mode 100644 index 0000000000..5b2618ebe4 --- /dev/null +++ b/src/test-setup.vscode.ts @@ -0,0 +1,111 @@ +import './test-setup'; +import { ResourceLoader } from '@angular/compiler'; +import { ɵresolveComponentResources } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; +import { beforeEach } from 'vitest'; +import { readdir, readFile } from 'node:fs/promises'; +import { join, normalize, sep } from 'node:path'; + +const workspaceRoot = process.cwd(); +const resourceRoots = [join(workspaceRoot, 'src')]; +let resourceIndexPromise: Promise> | null = null; + +class FileSystemResourceLoader extends ResourceLoader { + public async get(url: string): Promise { + const resourcePath = await resolveResourcePath(url); + return readFile(resourcePath, 'utf8'); + } +} + +async function buildResourceIndex(): Promise> { + const resourceIndex = new Map(); + + const addResourcePath = (absolutePath: string): void => { + const normalizedPath = normalize(absolutePath); + const relativePath = normalize(normalizedPath.replace(`${workspaceRoot}${sep}`, '')); + const segments = relativePath.split(sep); + const addKey = (key: string): void => { + const existingPaths = resourceIndex.get(key) ?? []; + + if (!existingPaths.includes(normalizedPath)) { + existingPaths.push(normalizedPath); + resourceIndex.set(key, existingPaths); + } + }; + + for (let i = 0; i < segments.length; i++) { + addKey(segments.slice(i).join('/')); + } + }; + + const walk = async (directory: string): Promise => { + const entries = await readdir(directory, { withFileTypes: true }); + + await Promise.all( + entries.map(async (entry) => { + const fullPath = join(directory, entry.name); + + if (entry.isDirectory()) { + await walk(fullPath); + return; + } + + if (entry.isFile() && (entry.name.endsWith('.html') || entry.name.endsWith('.scss'))) { + addResourcePath(fullPath); + } + }), + ); + }; + + await Promise.all(resourceRoots.map((root) => walk(root))); + + return resourceIndex; +} + +function getResourceIndex(): Promise> { + resourceIndexPromise ??= buildResourceIndex(); + return resourceIndexPromise; +} + +async function resolveResourcePath(url: string): Promise { + const normalizedUrl = normalize(url).replaceAll('\\', '/'); + const resourceKey = normalizedUrl + .split('/') + .filter((segment) => segment !== '.' && segment !== '..' && segment.length > 0) + .join('/'); + + if (!resourceKey) { + throw new Error(`Unable to resolve Angular component resource: "${url}"`); + } + + const resourceIndex = await getResourceIndex(); + const matches = resourceIndex.get(resourceKey) ?? []; + + if (matches.length === 1) { + return matches[0]; + } + + if (matches.length > 1) { + throw new Error( + `Ambiguous Angular component resource "${url}" matched multiple files:\n${matches.map((match) => ` - ${match}`).join('\n')}`, + ); + } + + throw new Error(`Unable to resolve Angular component resource: "${url}"`); +} + +async function loadAngularComponentResource(url: string): Promise { + const resourcePath = await resolveResourcePath(url); + return readFile(resourcePath, 'utf8'); +} + +TestBed.resetTestEnvironment(); +TestBed.initTestEnvironment( + BrowserDynamicTestingModule, + platformBrowserDynamicTesting([{ provide: ResourceLoader, useClass: FileSystemResourceLoader }]), +); + +beforeEach(async () => { + await ɵresolveComponentResources(loadAngularComponentResource); +}); diff --git a/vitest.config.ts b/vitest.config.ts index c67bab7426..a6ee6bfa33 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -18,6 +18,7 @@ export default defineConfig({ environment: 'jsdom', setupFiles: ['src/test-setup.ts'], exclude: [ + 'node_modules/**', 'cypress/**', '**/*.cy.ts', 'src/**/*.component.html', @@ -33,6 +34,7 @@ export default defineConfig({ reporter: ['text', 'html', 'lcov', 'json'], reportsDirectory: 'coverage', exclude: [ + 'node_modules/**', 'cypress/**', '**/*.cy.ts', 'src/**/*.component.html', diff --git a/vitest.vscode.config.ts b/vitest.vscode.config.ts new file mode 100644 index 0000000000..3ae8e7644d --- /dev/null +++ b/vitest.vscode.config.ts @@ -0,0 +1,12 @@ +import { defineConfig, mergeConfig } from 'vitest/config'; +import baseConfig from './vitest.config'; + +export default mergeConfig( + baseConfig, + defineConfig({ + test: { + include: ['src/**/*.spec.ts'], + setupFiles: ['src/test-setup.vscode.ts'], + }, + }), +); From 43f43aa9e502e77a630a33b78384967d710c5483 Mon Sep 17 00:00:00 2001 From: Frankie Moran Date: Tue, 31 Mar 2026 17:16:36 +0100 Subject: [PATCH 3/5] Update exclusion --- sonar-project.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonar-project.properties b/sonar-project.properties index 48dc55b29b..a3e57c61c1 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -6,6 +6,6 @@ sonar.sources=src/ sonar.tests=src/ sonar.exclusions=src/main.ts,src/main.server.ts,src/app/app.config.ts,src/app/app.config.server.ts,src/**/*.routes.ts,src/**/*.mock.ts sonar.test.inclusions=src/**/*.spec.ts -sonar.coverage.exclusions=src/**/*.spec.ts,src/test-setup.ts,src/main.ts,src/main.server.ts,src/app/app.config.ts,src/app/app.config.server.ts,src/**/*.component.html,src/**/*.mock.ts,src/**/*.interface.ts,src/**/*.type.ts,src/**/*.constant.ts,src/**/*.routes.ts,src/**/*.routing.ts +sonar.coverage.exclusions=src/**/*.spec.ts,src/test-setup.ts,src/test-setup.vscode.ts,src/main.ts,src/main.server.ts,src/app/app.config.ts,src/app/app.config.server.ts,src/**/*.component.html,src/**/*.mock.ts,src/**/*.interface.ts,src/**/*.type.ts,src/**/*.constant.ts,src/**/*.routes.ts,src/**/*.routing.ts sonar.javascript.lcov.reportPaths=coverage/lcov.info From 0abe3d5311b1e7d58e73ab727cd066a7dcb2beb7 Mon Sep 17 00:00:00 2001 From: Frankie Moran Date: Wed, 1 Apr 2026 09:42:15 +0100 Subject: [PATCH 4/5] Address code smells --- .../opal-frontend-review-guidelines/SKILL.md | 1 + src/test-setup.ts | 4 ++-- src/test-setup.vscode.ts | 12 ++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.codex/skills/opal-frontend/opal-frontend-review-guidelines/SKILL.md b/.codex/skills/opal-frontend/opal-frontend-review-guidelines/SKILL.md index 9a7cd99b76..12a20461ba 100644 --- a/.codex/skills/opal-frontend/opal-frontend-review-guidelines/SKILL.md +++ b/.codex/skills/opal-frontend/opal-frontend-review-guidelines/SKILL.md @@ -70,6 +70,7 @@ Apply these rules when reviewing changes in opal-frontend; focus on P0/P1 blocke - Add or maintain tests for new logic and error/empty states. - Prefer Angular Testing Library/Harnesses; avoid brittle DOM selectors/data-testids when a Harness exists. +- When a review finding concerns Vitest, Angular TestBed setup, render timing, or suite-load failures, consult `opal-vitest-guard` for the preferred diagnostic and fix patterns. ### Function Design diff --git a/src/test-setup.ts b/src/test-setup.ts index 393b37b235..cc95e4898e 100644 --- a/src/test-setup.ts +++ b/src/test-setup.ts @@ -1,6 +1,6 @@ import '@angular/compiler'; import { getTestBed } from '@angular/core/testing'; -import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; +import { BrowserTestingModule, platformBrowserTesting } from '@angular/platform-browser/testing'; import 'zone.js'; import 'zone.js/testing'; import { GovukRadioComponent } from '@hmcts/opal-frontend-common/components/govuk/govuk-radio'; @@ -9,7 +9,7 @@ import { MojDatePickerComponent } from '@hmcts/opal-frontend-common/components/m const testBed = getTestBed(); if (!testBed.platform) { - testBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting()); + testBed.initTestEnvironment(BrowserTestingModule, platformBrowserTesting()); } // JSDOM cannot initialize GOV.UK and MOJ frontend widgets used by shared components. diff --git a/src/test-setup.vscode.ts b/src/test-setup.vscode.ts index 5b2618ebe4..0e7526afe1 100644 --- a/src/test-setup.vscode.ts +++ b/src/test-setup.vscode.ts @@ -2,7 +2,7 @@ import './test-setup'; import { ResourceLoader } from '@angular/compiler'; import { ɵresolveComponentResources } from '@angular/core'; import { TestBed } from '@angular/core/testing'; -import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; +import { BrowserTestingModule, platformBrowserTesting } from '@angular/platform-browser/testing'; import { beforeEach } from 'vitest'; import { readdir, readFile } from 'node:fs/promises'; import { join, normalize, sep } from 'node:path'; @@ -87,9 +87,9 @@ async function resolveResourcePath(url: string): Promise { } if (matches.length > 1) { - throw new Error( - `Ambiguous Angular component resource "${url}" matched multiple files:\n${matches.map((match) => ` - ${match}`).join('\n')}`, - ); + const matchedFiles = matches.map((match) => ' - ' + match).join('\n'); + + throw new Error(`Ambiguous Angular component resource "${url}" matched multiple files:\n${matchedFiles}`); } throw new Error(`Unable to resolve Angular component resource: "${url}"`); @@ -102,8 +102,8 @@ async function loadAngularComponentResource(url: string): Promise { TestBed.resetTestEnvironment(); TestBed.initTestEnvironment( - BrowserDynamicTestingModule, - platformBrowserDynamicTesting([{ provide: ResourceLoader, useClass: FileSystemResourceLoader }]), + BrowserTestingModule, + platformBrowserTesting([{ provide: ResourceLoader, useClass: FileSystemResourceLoader }]), ); beforeEach(async () => { From 79fe6b86a23cd563077d43e4b189656b6cabb7a3 Mon Sep 17 00:00:00 2001 From: Frankie Moran Date: Wed, 1 Apr 2026 16:57:56 +0100 Subject: [PATCH 5/5] Add `disableConsoleIntercept: true,` --- vitest.config.ts | 1 + vitest.vscode.config.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/vitest.config.ts b/vitest.config.ts index a6ee6bfa33..2ea80a5497 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -16,6 +16,7 @@ export default defineConfig({ test: { globals: true, environment: 'jsdom', + disableConsoleIntercept: true, setupFiles: ['src/test-setup.ts'], exclude: [ 'node_modules/**', diff --git a/vitest.vscode.config.ts b/vitest.vscode.config.ts index 3ae8e7644d..cdbd7307d5 100644 --- a/vitest.vscode.config.ts +++ b/vitest.vscode.config.ts @@ -5,6 +5,7 @@ export default mergeConfig( baseConfig, defineConfig({ test: { + disableConsoleIntercept: true, include: ['src/**/*.spec.ts'], setupFiles: ['src/test-setup.vscode.ts'], },