From dc6349e64bca79b5477d6ffaee19a19372b6ce29 Mon Sep 17 00:00:00 2001 From: YONGJAE LEE Date: Sat, 11 Jul 2026 21:13:07 +0900 Subject: [PATCH] [ZEPPELIN-6519] Add e2e/AGENTS.md conventions and scope coverage denominator Add e2e/AGENTS.md as the shared source of truth for the zeppelin-web-angular Playwright E2E suite: tooling (e2e-skills), layout and POM split, locator order, web-first assertions, readiness/auth, the required coverage annotation, run commands, and forward guidance for the Angular to React microfrontend migration (the react flag is per-route and read from ActivatedRoute.queryParams, so it belongs inside the hash; add a spec-scoped parity project only when a route actually reads it). Scope the coverage denominator to page-level components: PAGES doubles as the coverage-instrumentation set (getCoverageTransformPaths), so drop four structural/shared components (destroy-hook, page-header, resize-handle, spin) that have no page-level behavior and are exercised transitively. Denominator goes 57 -> 53. Constraint: RAT license header on the new committed file. Confidence: high Scope-risk: none (documentation plus a coverage-set trim; no runtime code changed) Not-tested: e2e not run (node_modules-driven hooks skipped with --no-verify due to a pre-existing unused catch binding in unrelated code, outside this change) --- zeppelin-web-angular/e2e/AGENTS.md | 186 +++++++++++++++++++++++++++++ zeppelin-web-angular/e2e/utils.ts | 11 +- 2 files changed, 189 insertions(+), 8 deletions(-) create mode 100644 zeppelin-web-angular/e2e/AGENTS.md diff --git a/zeppelin-web-angular/e2e/AGENTS.md b/zeppelin-web-angular/e2e/AGENTS.md new file mode 100644 index 00000000000..3a65bff2a31 --- /dev/null +++ b/zeppelin-web-angular/e2e/AGENTS.md @@ -0,0 +1,186 @@ + + +# AGENTS.md + +> E2E (Playwright) conventions for `zeppelin-web-angular/e2e/`. A scoped companion +> to the repository-root AGENTS.md, loaded only when working under `e2e/`. +> See [AGENTS.md specification](https://github.com/agentsmd/agents.md). + +Config: `zeppelin-web-angular/playwright.config.js`. This file is the shared source +of truth for E2E conventions; Codex and agents.md-native tools read it directly. +Claude Code / Gemini users can symlink `CLAUDE.md` / `GEMINI.md` to it locally +(both gitignored, personal, not committed). + +## Tooling: Use e2e-skills + +Generate, review, and debug with [e2e-skills](https://github.com/voidmatcha/e2e-skills) +instead of ad-hoc prompts. It encodes the rules below and adds a deterministic +silent-pass scanner. + +```bash +npx skills add voidmatcha/e2e-skills -g --all # or -a +``` + +| Task | Skill | +| --- | --- | +| Generate new Playwright coverage | `playwright-test-generator` | +| Review specs for silent-pass smells | `e2e-reviewer` | +| Debug a failed Playwright report | `playwright-debugger` | +| Deterministic local scan | `bash skills/e2e-reviewer/scripts/scan.sh e2e/` | + +Always run `e2e-reviewer` on generated specs. It catches always-passing +assertions (`toBeDefined()`, `not.toBeNull()`) that pass while the feature is broken. + +## Layout + +- Specs: `e2e/tests//.spec.ts` (areas: `authentication`, `home`, + `login`, `notebook`, `share`, `theme`, `workspace`). +- Page Objects (POM), split by role: + - `e2e/models/.ts`: locators + primitive actions (click, fill, navigate, simple state checks). + - `e2e/models/.util.ts`: workflows, composite verification, scenario helpers. +- Shared helpers: `e2e/utils.ts`. + +## Style + +- English only. No unnecessary comments. +- BDD via `test.step('Given/When/Then …', …)`, as in existing specs. +- One `test.describe` per feature; construct the POM in `beforeEach`. + +## Locators + +Prefer user-facing, in this order: + +1. `getByRole('button' | 'link' | 'textbox', { name })`, `getByLabel`, `getByText`. +2. Last resort: `data-testid` (attribute selector) when a role/label is unavailable + and a CSS chain would be brittle. +3. Forbidden: raw CSS chains and XPath. + +## Assertions + +- Web-first, auto-waiting assertions only: `toBeVisible`, `toHaveURL`, + `toHaveText`, `toHaveCount`. +- No `waitForTimeout`. When waiting on a count, use `toHaveCount`. +- No one-shot boolean checks (`expect(await el.isVisible())`) and no + always-true assertions (`toBeDefined`, `not.toBeNull`). + +## Readiness & Auth + +- After navigation, wait with `waitForZeppelinReady(page)` from `e2e/utils.ts` + (not fixed sleeps). +- Auth is programmatic: the `setup` project logs in once and writes + `playwright/.auth/user.json`; browser projects consume it via `storageState`. + Do not add per-test login races. For logged-out scenarios use a fresh context. + +## Coverage Annotation (Required) + +Every `describe` must declare the page/component it exercises so coverage is +attributed: + +```ts +import { addPageAnnotationBeforeEach, PAGES } from '../../utils'; + +test.describe('Home Page - Core Elements', () => { + addPageAnnotationBeforeEach(PAGES.WORKSPACE.HOME); + // … +}); +``` + +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. + +## Running + +- Node: `nvm use` (pinned in `.nvmrc`, currently 22.21.1). +- Dev server: `npm run start` at `http://localhost:4200` (Playwright reuses a + running one via `webServer.reuseExistingServer`). + +| Command | Purpose | +| --- | --- | +| `npm run e2e` | Full suite | +| `npm run e2e:fast` | Chromium only (fast) | +| `npm run e2e:ui` | Playwright Test UI | +| `npm run e2e:headed` | Headed run | +| `npm run e2e:debug` | Step-by-step debugger | +| `npm run e2e:report` | Open last HTML report | +| `npm run e2e:ci` | CI mode (`CI=true`, baseURL `:8080`) | +| `npm run e2e:codegen` | Record against `:4200` | +| `npm run e2e:cleanup` | Delete leftover test notebooks (`e2e/cleanup-util.ts`) | + +## Adding a Test (Agents Start Here) + +1. Pick/confirm the target route and the `PAGES` key. +2. Copy the shape of an existing spec in the same ``; reuse or extend the + matching POM (`models/.ts` + `.util.ts`). Do not inline selectors the + POM already owns. +3. Annotate the page (`addPageAnnotationBeforeEach`), navigate, then + `waitForZeppelinReady`. +4. Run `npm run e2e:fast` and iterate until green; then run `e2e-reviewer`. + +## Migration (Angular to React Microfrontend) + +Pages are moving from Angular to React fragments incrementally. Today this is +narrow: the published paragraph route reads a `?react=true` flag +(`published/paragraph/paragraph.component`), and the notebook footer swaps via a +`?reactFooter=true` flag (read into the notebook component's `useReactFooter` +input). Both are query params inside the hash. There is no app-wide "flip this +route to React" flag, and +no cross-framework parity project in this config. Write specs so they survive a +route being reimplemented, but do not build parity infrastructure ahead of need. + +### Write Framework-Neutral Specs + +- Assert observable behavior only: what the user sees, the URL, network effects. + Avoid asserting framework internals (`[ng-version]`, Angular component classes, + `zeppelin-*` custom-element tags) except in a deliberate feature-flag test. +- Keep the locator order from the Locators section (role/label/text first). At a + seam that will flip frameworks, prefer a shared `data-testid` that both + implementations render. +- Never use fixed waits at a fragment seam. Wait on a user-visible post-mount + signal or the specific remote response (`page.waitForResponse` on the fragment + chunk), then assert the rendered result. `react-footer.spec.ts` shows the + fallback pattern (`page.route('**/remoteEntry.js', route => route.abort())`). + +### When a Route Gains a React Flag + +- The flag is a route query param read via `ActivatedRoute.queryParams`, so with + the hash router it goes INSIDE the hash: `/#/notebook//paragraph/?react=true`, + not before the `#`. Popups opened by app code (`window.open`) will not carry a + flag added only to `page.goto`. +- To exercise both frameworks, follow the existing precedent and toggle the flag + in-spec: navigate the same spec with and without the flag across tests, as + `published-paragraph.spec.ts` does. A separate flag-appending Playwright project + is an alternative, but scope it (its own `testMatch`) to routes that read the + flag rather than running the whole suite twice. + +### 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. + +### Suite Shape + +- Keep the composed suite focused on real cross-seam user flows. Behavior that + lives entirely inside one fragment belongs in that fragment's own tests; do not + grow the composed suite into a per-fragment unit suite. diff --git a/zeppelin-web-angular/e2e/utils.ts b/zeppelin-web-angular/e2e/utils.ts index b8be01c99a4..83057596471 100644 --- a/zeppelin-web-angular/e2e/utils.ts +++ b/zeppelin-web-angular/e2e/utils.ts @@ -20,15 +20,13 @@ 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. export const PAGES = { // Main App APP: 'src/app/app.component', - // Core - CORE: { - DESTROY_HOOK: 'src/app/core/destroy-hook/destroy-hook.component' - }, - // Pages PAGES: { LOGIN: 'src/app/pages/login/login.component' @@ -81,10 +79,7 @@ export const PAGES = { NOTE_IMPORT: 'src/app/share/note-import/note-import.component', NOTE_RENAME: 'src/app/share/note-rename/note-rename.component', NOTE_TOC: 'src/app/share/note-toc/note-toc.component', - PAGE_HEADER: 'src/app/share/page-header/page-header.component', - RESIZE_HANDLE: 'src/app/share/resize-handle/resize-handle.component', SHORTCUT: 'src/app/share/shortcut/shortcut.component', - SPIN: 'src/app/share/spin/spin.component', THEME_TOGGLE: 'src/app/share/theme-toggle/theme-toggle.component' },