PO-3541

.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
 

.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.

.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
+});
+```

.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",
   ]
 }

.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",

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

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);

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();
   });
 

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();

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.

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.

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',

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';

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,

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,

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';