[ZEPPELIN-6686] Align E2E coverage with Angular component targets

zeppelin-web-angular/AGENTS.md

@@ -17,23 +17,23 @@ limitations under the License.
 
 # AGENTS.md
 
-Unit test conventions for this package. They apply to the Angular shell in `src/` and to the libraries under `projects/` that have no file of their own: `zeppelin-sdk`, which is framework-neutral, and `zeppelin-visualization`, which is mostly so apart from one `@Component` base class.
+Unit test conventions for this package. They apply to the Angular shell in `src/`, package-level infrastructure specs under `test/`, and the libraries under `projects/` that have no file of their own: `zeppelin-sdk`, which is framework-neutral, and `zeppelin-visualization`, which is mostly so apart from one `@Component` base class.
 
 Two subtrees override this file: [`e2e/AGENTS.md`](e2e/AGENTS.md) for the Playwright suite, and [`projects/zeppelin-react/AGENTS.md`](projects/zeppelin-react/AGENTS.md) for the React remote, which has a different CI status and one exception of its own.
 
 The repository root `AGENTS.md` asks every change to include unit tests. This file is how.
 
 ## Layout
 
-- A spec lives next to its source: `foo.ts` / `foo.spec.ts`.
-- The runner is Vitest on jsdom. There is no Karma and no `TestBed` bootstrap in the setup file; `test/test-setup.ts` loads `zone.js` and nothing else.
-- `npm run test:shell` covers `src/`, `projects/zeppelin-sdk` and `projects/zeppelin-visualization`. The two libraries have no runner of their own; they ride on the shell config because their code needs nothing extra. `projects/zeppelin-react` is separate. It has its own Vitest config and its own file here.
+- A product-code spec lives next to its source: `foo.ts` / `foo.spec.ts`. Specs for package-level test and reporting infrastructure live under `test/`.
+- The runner is Vitest on jsdom. There is no Karma. `test/test-setup.ts` loads `zone.js` and reflection metadata, initializes Angular `TestBed`, and resets the test environment after each spec.
+- `npm run test:shell` covers `src/`, package-level specs under `test/`, `projects/zeppelin-sdk` and `projects/zeppelin-visualization`. The two libraries have no runner of their own; they ride on the shell config because their code needs nothing extra. `projects/zeppelin-react` is separate. It has its own Vitest config and its own file here.
 
 ## Running
 
 | Command | Purpose |
 | --- | --- |
-| `npm run test:shell` | Run the unit tests for `src/` and the two libraries |
+| `npm run test:shell` | Run the unit tests for `src/`, `test/` and the two libraries |
 | `npm run test:shell -- --coverage` | Same, with a coverage report |
 | `npm run test:shell -- foo.spec.ts` | Run one file |
 
@@ -97,9 +97,9 @@ it('renders a dash for null and undefined', ...)     // good
 it('handles input', ...)                             // not a test name
 ```
 
-## Angular classes without TestBed
+## Angular classes with and without TestBed
 
-Directives, pipes and services are plain classes. Construct them directly and pass mocks to the constructor. `TestBed` is not needed and is slower.
+Construct directly testable directives, pipes and services as plain classes when Angular framework wiring is not the subject of the spec. Pass mocks to the constructor instead of starting `TestBed` unnecessarily.
 
 ```ts
 const loader = { loadModule } as Pick<ReactRemoteLoaderService, 'loadModule'>;
@@ -108,7 +108,7 @@ const directive = new ReactMountDirective(host, ngZone, loader as ReactRemoteLoa
 
 See `src/app/share/react-mount/react-mount.directive.spec.ts` for a worked example, and `src/app/share/pipes/humanize-bytes.pipe.spec.ts` for a pipe.
 
-**A spec cannot declare a decorator of its own.** Importing a decorated class from source works (the pipe and directive specs here do exactly that), but writing `@Injectable()` inside a spec file fails with `SyntaxError: Invalid or unexpected token`. Spec files are excluded from the nearest `tsconfig.json` (`src/tsconfig.json` for the shell, each library's own under `projects/`), so the transform never picks up the decorator settings; that is [ZEPPELIN-6637](https://issues.apache.org/jira/browse/ZEPPELIN-6637). Until it lands, test a class by constructing it rather than by declaring a stand-in. Conventions for TestBed-based component specs are added here once it does.
+Use `TestBed` when the behavior depends on template bindings, dependency injection, change detection, or Angular lifecycle wiring. `src/app/share/react-mount/react-mount.directive.testbed.spec.ts` shows that path with a decorated host component. Keep the direct-construction spec beside it for behavior that does not need Angular wiring.
 
 ## Migration (Angular to React)
 
@@ -122,9 +122,9 @@ A spec written after a surface moves to React pins the new implementation's beha
 
 `--coverage` produces a v8 report under `coverage/`. It is measured, not gated. There are no thresholds.
 
-**Read the percentage carefully: it is not whole-tree coverage.** The denominator is only the files the specs actually load, because `include` is left unset while ZEPPELIN-6637 is open (see `vitest.shell.config.mts`). Most of the tree is absent from the report rather than counted as zero, so the figure reads far better than the real state, and it can *fall* as specs are added, since each new spec pulls more files into the denominator. Expect a large drop when `include` is eventually turned on.
+**Read the percentage carefully: it is not whole-tree coverage.** The denominator is only the files the specs actually load, because `include` is left unset (see `vitest.shell.config.mts`). Coverage parsing is separate from the test transform, so the whole tree must be deliberately remeasured before that setting is widened. Most of the tree is absent from the report rather than counted as zero, so the figure reads far better than the real state, and it can *fall* as specs are added, since each new spec pulls more files into the denominator. Expect a large drop when `include` is eventually turned on.
 
-That is deliberate. `src/` currently holds a few specs against 222 source files, so any threshold set today is either meaningless or permanently red. The intended progression is: measure only, then ratchet so the number cannot fall, then require coverage on changed files. A whole-tree percentage is the wrong target during a migration, because much of the tree is going to be rewritten anyway.
+That is deliberate. `src/` currently holds a few specs against more than 200 source files, so any threshold set today is either meaningless or permanently red. The intended progression is: measure only, then ratchet so the number cannot fall, then require coverage on changed files. A whole-tree percentage is the wrong target during a migration, because much of the tree is going to be rewritten anyway.
 
 This is a different measurement from `e2e/reporter.coverage.ts`, which counts annotated component pages rather than lines. The two numbers are not comparable and are not merged.
 
@@ -134,5 +134,5 @@ This is a different measurement from `e2e/reporter.coverage.ts`, which counts an
 2. Create `foo.spec.ts` next to `foo.ts`.
 3. Import from `vitest` (`describe`, `expect`, `it`), not from Jasmine or Jest.
    Check the target has callers before you invest in it. `get-keyword-positions.spec.ts` is a worked example of a function that turned out to have none.
-4. Construct the class directly; do not reach for `TestBed`.
+4. Construct the class directly unless the behavior depends on Angular wiring; use `TestBed` when it does.
 5. Run `npm run test:shell` and confirm it passes before opening a PR.

zeppelin-web-angular/angular.json

@@ -151,7 +151,7 @@
         "lint": {
           "builder": "@angular-eslint/builder:lint",
           "options": {
-            "lintFilePatterns": ["src/**/*.ts", "src/**/*.html", "e2e/**/*.ts"]
+            "lintFilePatterns": ["src/**/*.ts", "src/**/*.html", "test/**/*.ts", "e2e/**/*.ts"]
           }
         }
       }

zeppelin-web-angular/e2e/AGENTS.md

@@ -88,7 +88,7 @@ test.describe('Home Page - Core Elements', () => {
 });
 ```
 
-Use an existing key from the `PAGES` object in `e2e/utils.ts`; add a new one there if the page is missing. `PAGES` is also the coverage-instrumentation set (`getCoverageTransformPaths`), so it defines the coverage denominator. Purely structural / non-page components (lifecycle hooks, shared UI primitives like the spinner or resize handle) are intentionally omitted from `PAGES`. They are exercised transitively and are not counted.
+Use an existing key from the `PAGES` object in `e2e/utils.ts`; add a new one there if the page is missing. The reporter discovers `src/app/**/*.component.ts` automatically and removes only the entries in `COVERAGE_EXCLUDED_COMPONENTS`, so component additions, deletions and moves update the denominator automatically. `PAGES` separately supplies the annotation names and must match those discovered targets. `test/reporter.coverage.spec.ts` enforces that match, rejects duplicate entries and verifies that every explicit exclusion still exists. Purely structural / non-page components (lifecycle hooks, shared UI primitives like the spinner or resize handle) are exercised transitively and are not counted.
 
 ## Running
 
@@ -136,7 +136,7 @@ Pages are moving from Angular to React fragments incrementally. Today this is na
 
 ### Coverage
 
-- Coverage is tracked by `PAGES` key, not source file. The key is the stable identity; the path behind it is an implementation detail. When a page moves to React, update its path in `PAGES` rather than deleting the key (deleting drops it from the coverage denominator). Specs keep the same `addPageAnnotationBeforeEach(PAGES.KEY)` call across the migration.
+- Coverage attribution is tracked by `PAGES` key while the denominator is discovered from the Angular component tree. The key is the stable identity; the path behind it is an implementation detail. While Angular still hosts the route, keep the key mapped to that host component and cover fragment-only behavior in the React package tests. When the Angular host component is removed, revise the composed E2E target policy in the same change instead of silently deleting the key. Specs keep the same `addPageAnnotationBeforeEach(PAGES.KEY)` call across the migration.
 
 ### Suite Shape
 
@@ -148,7 +148,7 @@ Pages are moving from Angular to React fragments incrementally. Today this is na
 
 - **Locators (classic exception):** the classic templates predate roles and `data-testid`, so the role/label/text-first rule cannot apply. Sanctioned here: element ids (`#findInput`), `ng-click="..."` / `ng-controller="..."` attribute selectors, class selectors the legacy templates already expose (`.username`, `.interpreterHead`), and Ace/Select2 internals. Do not add `data-testid` to the frozen `zeppelin-web` sources.
 - **Readiness:** `waitForZeppelinReady` is Angular-specific (`[ng-version]`) and does not resolve on `/classic`; gate on a classic-visible signal instead (e.g. the first `ParagraphCtrl` paragraph, or `.ace_text-input` attached).
-- **Coverage:** `PAGES` is the Angular coverage denominator; classic pages are intentionally outside it, so `addPageAnnotationBeforeEach` is not used here.
+- **Coverage:** classic pages are outside the discovered Angular component target set, so `addPageAnnotationBeforeEach` is not used here.
 - **Running:** the classic suite has its own config, `playwright.classic.config.js` (Desktop Chrome only, targets `http://localhost:8080`), and needs a Zeppelin server built with `-Pweb-classic`. The `:4200` dev server does not serve `/classic`, so a plain `npm run e2e` never includes it. Run it with `npm run e2e:classic` (single spec: `npm run e2e:classic -- tests/classic/<spec>`). In CI the workflow enables it on the anonymous matrix leg only (`-Dweb.e2e.classic.disabled=false`), matching the anonymous-only legacy Protractor suite.
 - **POM:** inlining locators/helpers is acceptable while the suite is this small; if it grows, move them behind `models/classic-*.ts` / `*.util.ts`.
 - The React-migration / framework-neutral-spec guidance does not apply to `tests/classic/`.

zeppelin-web-angular/e2e/reporter.coverage.config.ts

@@ -16,11 +16,7 @@ import { getCoverageTransformPaths } from './utils';
 const outputPath = join(__dirname, '..', 'playwright-coverage');
 
 export default {
-  rootPath: join(__dirname, '..'),
   outputPath,
-  testMatch: [/\.component$/],
-  excludes: [/\.spec\.ts$/, /\.module\.ts$/, /\.guard\.ts$/, /\.routing\.ts$/, /\.html$/, /\.less$/, /\.css$/],
-  // Transform configuration for coverage instrumentation
-  // Specifies which component files to instrument for coverage tracking
+  // Automatically discovered component targets are the reporter denominator.
   transform: getCoverageTransformPaths()
 };

zeppelin-web-angular/e2e/reporter.coverage.ts

@@ -15,7 +15,6 @@ import { promises as fs } from 'fs';
 import { join } from 'path';
 import { FullResult, Reporter, TestCase, TestResult } from '@playwright/test/reporter';
 import { flatMap, sortBy } from 'lodash';
-import { scanDirectory, Results } from 'scandirectory';
 import cfg from './reporter.coverage.config';
 
 const TEST_STATUS = {
@@ -47,15 +46,9 @@ class CoverageReporter implements Reporter {
   testedIds = new Map<string, TestStatusType>();
   targetPaths: string[] = [];
 
-  async onBegin() {
+  onBegin() {
     console.log('Coverage reporter starting...');
-    console.log('Root path:', cfg.rootPath);
-
-    const results = await scanDirectory({
-      directory: cfg.rootPath
-    });
-
-    this.targetPaths = this.processScannedFiles(results);
+    this.targetPaths = [...cfg.transform];
     console.log('Target paths:', this.targetPaths.length);
   }
 
@@ -85,39 +78,6 @@ class CoverageReporter implements Reporter {
     await this.saveResultsToFile(results, result.status);
   }
 
-  processScannedFiles(results: Results): string[] {
-    return Object.keys(results)
-      .filter(key => !results[key].directory)
-      .map(key => this.normalizeFilePath(key, results))
-      .filter(key => key !== '.')
-      .filter(key => this.shouldIncludeFile(key));
-  }
-
-  normalizeFilePath(key: string, results: Results): string {
-    if (/index\.tsx?$/.test(key)) {
-      return results[key].parent?.relativePath || '.';
-    }
-    return key.replace(/\.tsx?$/, '');
-  }
-
-  shouldIncludeFile(key: string): boolean {
-    if (cfg.testMatch?.length) {
-      const matchesTest = cfg.testMatch.some(rule => (rule instanceof RegExp ? rule.test(key) : rule === key));
-      if (!matchesTest) {
-        return false;
-      }
-    }
-
-    if (cfg.excludes?.length) {
-      const isExcluded = cfg.excludes.some(rule => (rule instanceof RegExp ? rule.test(key) : rule === key));
-      if (isExcluded) {
-        return false;
-      }
-    }
-
-    return true;
-  }
-
   extractPageAnnotations(test: TestCase): string[] {
     const annotations = test.annotations
       .filter(({ type }) => type === 'page')

zeppelin-web-angular/e2e/utils.ts

@@ -10,6 +10,8 @@
  * limitations under the License.
  */
 
+import { globSync } from 'fs';
+import { join, sep } from 'path';
 import { test, expect, Page, TestInfo } from '@playwright/test';
 import { LoginTestUtil } from './models/login-page.util';
 import { E2E_TEST_FOLDER } from './models/base-page';
@@ -20,9 +22,7 @@ export const NOTEBOOK_PATTERNS = {
   LINK_SELECTOR: 'a[href*="/notebook/"]'
 } as const;
 
-// Coverage denominator. Structural/shared components
-// (lifecycle hooks, spin, resize-handle, page-header) are intentionally omitted;
-// they have no page-level behavior and are exercised transitively.
+// Annotation registry for page-level coverage.
 export const PAGES = {
   // Main App
   APP: 'src/app/app.component',
@@ -99,6 +99,14 @@ export const PAGES = {
   }
 } as const;
 
+// These structural/shared components have no page-level behavior and are exercised transitively rather than counted as separate E2E targets.
+export const COVERAGE_EXCLUDED_COMPONENTS = [
+  'src/app/core/destroy-hook/destroy-hook.component',
+  'src/app/share/page-header/page-header.component',
+  'src/app/share/resize-handle/resize-handle.component',
+  'src/app/share/spin/spin.component'
+] as const;
+
 export const addPageAnnotation = (pageName: string, testInfo: TestInfo) => {
   testInfo.annotations.push({
     type: 'page',
@@ -133,7 +141,17 @@ export const flattenPageComponents = (pages: PageStructureType): string[] => {
   return result.sort();
 };
 
-export const getCoverageTransformPaths = (): string[] => flattenPageComponents(PAGES);
+export const getCoverageTransformPaths = (
+  rootPath = join(__dirname, '..'),
+  excludedComponents: readonly string[] = COVERAGE_EXCLUDED_COMPONENTS
+): string[] => {
+  const excluded = new Set(excludedComponents);
+
+  return globSync('src/app/**/*.component.ts', { cwd: rootPath })
+    .map(componentPath => componentPath.split(sep).join('/').replace(/\.ts$/, ''))
+    .filter(componentPath => !excluded.has(componentPath))
+    .sort();
+};
 
 export const waitForUrlNotContaining = async (page: Page, fragment: string) => {
   await page.waitForLoadState('domcontentloaded', { timeout: 10000 });

zeppelin-web-angular/eslint.config.js

@@ -164,6 +164,7 @@ module.exports = tseslint.config(
     files: [
       'src/**/*.spec.ts',
       'projects/zeppelin-{sdk,visualization}/**/*.spec.ts',
+      'test/**/*.spec.ts',
       'test/test-setup.ts',
       'vitest.shell.config.mts'
     ],
@@ -176,7 +177,7 @@ module.exports = tseslint.config(
   },
   {
     // Catch specs that cannot fail, as eslint-plugin-playwright does for e2e.
-    files: ['src/**/*.spec.ts', 'projects/zeppelin-{sdk,visualization}/**/*.spec.ts'],
+    files: ['src/**/*.spec.ts', 'projects/zeppelin-{sdk,visualization}/**/*.spec.ts', 'test/**/*.spec.ts'],
     plugins: { vitest },
     rules: {
       'vitest/expect-expect': 'error',

zeppelin-web-angular/package.json

@@ -109,7 +109,6 @@
     "monaco-editor-webpack-plugin": "7.1.1",
     "ng-packagr": "^21.2.3",
     "prettier": "^3.6.2",
-    "scandirectory": "8.1.1",
     "style-loader": "^4.0.0",
     "ts-node": "~7.0.0",
     "typescript": "~5.9.3",