Unit tests updates (#915)

Update unit tests after upgrade.

Author: nsemets

.github/scripts/check-coverage-thresholds.js

@@ -1,4 +1,3 @@
-const fs = require('fs');
 const coverage = require('../../coverage/coverage-summary.json');
 const jestConfig = require('../../jest.config.js');
 
@@ -43,7 +42,7 @@ if (failed) {
   console.log('\n\nCongratulations! You have successfully run the coverage check and added tests.');
   console.log('\n\nThe jest.config.js file is not insync with your new test additions.');
   console.log('Please update the coverage thresholds in jest.config.js.');
-  console.log('You will need to commit again once you have updated the jst.config.js file.');
+  console.log('You will need to commit again once you have updated the jst.config.ts file.');
   console.log('This is only necessary until we hit 100% coverage.');
   console.log(`\n\n${stars}`);
   errors.forEach((err) => {

docs/testing.md

@@ -45,7 +45,6 @@
 ```
 src/testing/
 ├── osf.testing.provider.ts       ← provideOSFCore(), provideOSFHttp()
-├── osf.testing.module.ts         ← OSFTestingModule (legacy — prefer providers)
 ├── providers/                    ← Builder-pattern mocks for services
 │   ├── store-provider.mock.ts
 │   ├── route-provider.mock.ts
@@ -68,21 +67,14 @@ src/testing/
 
 ### `provideOSFCore()` — mandatory base provider
 
-Every component test must include `provideOSFCore()`. It configures animations, translations, and environment tokens.
+Every component test must include `provideOSFCore()`. It configures translations and environment tokens.
 
 ```typescript
 export function provideOSFCore() {
-  return [
-    provideNoopAnimations(),
-    importProvidersFrom(TranslateModule.forRoot()),
-    TranslationServiceMock,
-    EnvironmentTokenMock,
-  ];
+  return [provideTranslation, TranslateServiceMock, EnvironmentTokenMock];
 }
 ```
 
-> **Never** import `OSFTestingModule` directly in new tests. It is retained for legacy compatibility only. Use `provideOSFCore()` instead.
-
 ---
 
 ## 3. Test File Structure
@@ -319,7 +311,35 @@ expect(store.dispatch).toHaveBeenCalledWith(new SpecificAction());
 
 ## 7. Router & Route Mocking
 
-### ActivatedRoute
+Use this checklist:
+
+### `provideRouter([])`
+
+- Use when the template/component needs router infrastructure (`routerLink`, `routerLinkActive`, router directives/providers).
+- Keep it local to the spec.
+- Skip it for pure logic tests without router directive usage.
+
+### `ActivatedRouteMockBuilder`
+
+- Use when code reads route state (`snapshot.paramMap`, `params`, `queryParams`, `parent`, and similar route inputs).
+- Use it for deterministic route inputs in unit tests.
+- Works with or without `provideRouter([])` depending on template needs.
+
+### `RouterMockBuilder`
+
+- Use when you assert navigation calls (`navigate`, `navigateByUrl`, `url`/events usage).
+- Best for behavior assertions, not real router integration.
+- If you mock `Router`, you test whether navigation was requested, not real routing execution.
+- Do not mock `Router` in specs that validate real routing behavior via `provideRouter(...)`.
+
+### Typical combos
+
+- Params only: `ActivatedRouteMockBuilder`
+- Params + navigation assertions: `ActivatedRouteMockBuilder` + `RouterMockBuilder`
+- Template has `routerLink` + params + navigation assertions: `provideRouter([])` + `ActivatedRouteMockBuilder` + `RouterMockBuilder`
+- Need real router behavior: `provideRouter(...)` + `ActivatedRoute` setup, avoid mocking `Router`
+
+### `ActivatedRouteMockBuilder` examples
 
 ```typescript
 const mockRoute = ActivatedRouteMockBuilder.create()
@@ -338,7 +358,7 @@ const mockRoute = ActivatedRouteMockBuilder.create()
 const mockRoute = ActivatedRouteMockBuilder.create().withParams({ id: 'reg-1' }).withNoParent().build();
 ```
 
-### Router
+### `RouterMockBuilder` examples
 
 ```typescript
 const mockRouter = RouterMockBuilder.create().withUrl('/registries/drafts/reg-1/metadata').build();
@@ -421,37 +441,57 @@ fixture.detectChanges();
 
 ## 10. Async Operations
 
-### `fakeAsync` + `tick` for debounced operations
+### Zoneless change detection
+
+In a zoneless environment, `fixture.detectChanges()` is used for immediate synchronous rendering. For signal updates and other async logic, use `await fixture.whenStable()` before DOM assertions so the scheduler can finish.
+
+```typescript
+it('should update UI after signal change', async () => {
+  mySignal.set(newVal);
+  await fixture.whenStable();
+  expect(fixture.nativeElement.textContent).toContain(newVal);
+});
+```
+
+### Debounced operations with Jest timers
 
 ```typescript
-it('should dispatch after debounce', fakeAsync(() => {
+it('should dispatch after debounce', () => {
+  jest.useFakeTimers();
   (store.dispatch as jest.Mock).mockClear();
+
   component.onProjectFilter('abc');
-  tick(300);
+  jest.advanceTimersByTime(300);
+
   expect(store.dispatch).toHaveBeenCalledWith(new GetProjects('user-1', 'abc'));
-}));
+  jest.useRealTimers();
+});
 
-// Deduplication — only the last value dispatches
-it('should debounce rapid calls', fakeAsync(() => {
+it('should debounce rapid calls', () => {
+  jest.useFakeTimers();
   (store.dispatch as jest.Mock).mockClear();
+
   component.onProjectFilter('a');
   component.onProjectFilter('ab');
   component.onProjectFilter('abc');
-  tick(300);
-  const calls = (store.dispatch as jest.Mock).mock.calls.filter(([a]: [any]) => a instanceof GetProjects);
+  jest.advanceTimersByTime(300);
+
+  const calls = (store.dispatch as jest.Mock).mock.calls.filter(([a]: [unknown]) => a instanceof GetProjects);
   expect(calls.length).toBe(1);
-}));
+  jest.useRealTimers();
+});
 ```
 
-### `done` callback for output emissions
+### Output emissions with explicit async flow
 
 ```typescript
-it('should emit attachFile', (done) => {
-  component.attachFile.subscribe((f) => {
-    expect(f).toEqual({ id: 'file-1' });
-    done();
+it('should emit attachFile', async () => {
+  const emitted = new Promise<FileModel>((resolve) => {
+    component.attachFile.subscribe((file) => resolve(file));
   });
+
   component.selectFile({ id: 'file-1' } as FileModel);
+  await expect(emitted).resolves.toEqual({ id: 'file-1' });
 });
 ```
 
@@ -854,7 +894,7 @@ This project strictly enforces 90%+ test coverage through GitHub Actions CI.
 
 ## 18. Best Practices
 
-1. **Always use `provideOSFCore()`** — never import `OSFTestingModule` directly in new tests.
+1. **Always use `provideOSFCore()`**.
 2. **Always use `provideMockStore()`** — never mock `component.actions` via `Object.defineProperty`.
 3. **Always pass explicit mocks to `MockProvider`** when you need `jest.fn()` assertions. Bare `MockProvider(Service)` creates ng-mocks stubs.
 4. **Check `@testing/` before creating inline mocks** — builders and factories almost certainly exist.
@@ -863,13 +903,12 @@ This project strictly enforces 90%+ test coverage through GitHub Actions CI.
 7. **Use `(store.dispatch as jest.Mock).mockClear()`** when `ngOnInit` dispatches and you need isolated per-test assertions.
 8. **Use `WritableSignal` for dynamic state** — pass `signal()` values to `provideMockStore` when tests need to mutate state mid-test.
 9. **Use `Subject` for dialog `onClose`** — gives explicit control over dialog result timing. Use `provideDynamicDialogRefMock()` where applicable.
-10. **Use `fakeAsync` + `tick`** for debounced operations — specify the exact debounce duration.
+10. **Use Jest fake timers** for debounced operations — `jest.useFakeTimers()`, `jest.advanceTimersByTime(ms)`, and `jest.useRealTimers()`.
 11. **Use `fixture.componentRef.setInput()`** for signal inputs — never direct property assignment.
-12. **Use `ngMocks.faster()`** when all tests in a file share identical `TestBed` config — reuses the compiled module for speed. Do not use if any test requires a different config: shared state will cause subtle test pollution.
-13. **Use typed mock interfaces** (`ToastServiceMockType`, `RouterMockType`, etc.) — avoid `any`.
-14. **Test both positive and negative paths** — confirm an action fires AND confirm it does not fire when conditions are not met.
-15. **Only use `@testing/data` fixtures in HTTP flushes** — never hardcode response values inline in service or state tests.
-16. **Each test should highlight the most critical aspect of the code** — if a test fails during a refactor, it should clearly signal that a core feature was impacted.
+12. **Use typed mock interfaces** (`ToastServiceMockType`, `RouterMockType`, etc.) — avoid `any`.
+13. **Test both positive and negative paths** — confirm an action fires AND confirm it does not fire when conditions are not met.
+14. **Only use `@testing/data` fixtures in HTTP flushes** — never hardcode response values inline in service or state tests.
+15. **Each test should highlight the most critical aspect of the code** — if a test fails during a refactor, it should clearly signal that a core feature was impacted.
 
 ---
 
@@ -902,7 +941,7 @@ expect(callArgs[1].data.draftId).toBe('draft-1');
 ### Filtering dispatch calls by action type
 
 ```typescript
-const calls = (store.dispatch as jest.Mock).mock.calls.filter(([a]: [any]) => a instanceof GetProjects);
+const calls = (store.dispatch as jest.Mock).mock.calls.filter(([a]: [unknown]) => a instanceof GetProjects);
 expect(calls.length).toBe(1);
 expect(calls[0][0]).toEqual(new GetProjects('user-1', 'abc'));
 ```

jest.config.js

@@ -1,11 +1,16 @@
-module.exports = {
+const config = {
   preset: 'jest-preset-angular',
   setupFilesAfterEnv: ['<rootDir>/setup-jest.ts'],
   globalSetup: '<rootDir>/jest.global-setup.ts',
-  collectCoverage: false,
+  testEnvironment: 'jsdom',
   clearMocks: true,
   restoreMocks: true,
-  coverageReporters: ['json-summary', 'lcov', 'clover'],
+  collectCoverage: false,
+  coverageDirectory: 'coverage',
+  coverageReporters: ['json-summary', 'lcov', 'clover', 'text-summary'],
+  moduleFileExtensions: ['ts', 'js', 'html', 'json', 'mjs'],
+  extensionsToTreatAsEsm: ['.ts'],
+  testMatch: ['<rootDir>/src/**/*.spec.ts'],
   moduleNameMapper: {
     '^@osf/(.*)$': '<rootDir>/src/app/$1',
     '^@core/(.*)$': '<rootDir>/src/app/core/$1',
@@ -27,11 +32,8 @@ module.exports = {
     ],
   },
   transformIgnorePatterns: [
-    'node_modules/(?!.*\\.mjs$|@ngxs|@angular|@ngrx|parse5|entities|chart.js|@mdit|@citation-js|@traptitech|@sentry|@primeng|@newrelic)',
+    'node_modules/(?!(@angular|@ngxs|@ngx-translate|angular-google-tag-manager|ngx-cookie-service|ngx-markdown-editor|angularx-qrcode|ngx-captcha|@sentry|@newrelic|@centerforopenscience|@mdit|@traptitech|@citation-js|primeng|@primeuix|markdown-it|markdown-it-anchor|markdown-it-toc-done-right|markdown-it-video|chart\\.js)/)',
   ],
-  testEnvironment: 'jsdom',
-  moduleFileExtensions: ['ts', 'js', 'html', 'json', 'mjs'],
-  coverageDirectory: 'coverage',
   collectCoverageFrom: [
     'src/app/**/*.{ts,js}',
     '!src/app/core/theme/**',
@@ -40,24 +42,21 @@ module.exports = {
     '!src/app/**/*.routes.{ts,js}',
     '!src/app/**/*.route.{ts,js}',
     '!src/app/**/mappers/**',
-    '!src/app/shared/mappers/**',
     '!src/app/**/*.model.{ts,js}',
     '!src/app/**/models/*.{ts,js}',
-    '!src/app/shared/models/**',
     '!src/app/**/*.enum.{ts,js}',
     '!src/app/**/*.type.{ts,js}',
     '!src/app/**/*.spec.{ts,js}',
     '!src/app/**/*.module.ts',
     '!src/app/**/index.ts',
     '!src/app/**/public-api.ts',
   ],
-  extensionsToTreatAsEsm: ['.ts'],
   coverageThreshold: {
     global: {
       branches: 43.3,
-      functions: 42.7,
-      lines: 69.3,
-      statements: 69.8,
+      functions: 43.8,
+      lines: 70.18,
+      statements: 70.6,
     },
   },
   watchPathIgnorePatterns: [
@@ -68,11 +67,7 @@ module.exports = {
     '<rootDir>/src/environments/',
     '<rootDir>/src/@types/',
   ],
-  testPathIgnorePatterns: [
-    '<rootDir>/src/environments',
-    '<rootDir>/src/app/features/files/pages/file-detail',
-    '<rootDir>/src/app/features/project/addons/',
-    '<rootDir>/src/app/features/settings/addons/',
-    '<rootDir>/src/app/features/settings/tokens/store/',
-  ],
+  testPathIgnorePatterns: ['<rootDir>/src/environments'],
 };
+
+module.exports = config;