Skip to content

[ZEPPELIN-6648] Add unit test conventions and coverage reporting for the new UI - #5433

Open
voidmatcha wants to merge 1 commit into
apache:masterfrom
voidmatcha:unit-test-conventions
Open

[ZEPPELIN-6648] Add unit test conventions and coverage reporting for the new UI#5433
voidmatcha wants to merge 1 commit into
apache:masterfrom
voidmatcha:unit-test-conventions

Conversation

@voidmatcha

@voidmatcha voidmatcha commented Aug 22, 2026

Copy link
Copy Markdown
Member

What is this PR for?

ZEPPELIN-6567 gave zeppelin-web-angular a self-contained Vitest/jsdom setup, and test:shell runs in the Maven test phase. Almost nothing runs on it: src/ holds 222 source files and two specs, and the five specs under projects/zeppelin-react cover theme helpers and two components.

The root AGENTS.md asks every change to include unit tests without saying how to write one, there is no coverage reporting on the unit side, and nothing stops a spec that cannot fail from being merged — the e2e suite gets that protection from eslint-plugin-playwright, the unit side had no equivalent.

This adds the conventions, the measurement, the enforcement, and three example specs so the conventions have something to point at.

  • AGENTS.md for the package, following the shape of e2e/AGENTS.md. It is the baseline for src/ and for the two libraries under projects/ that have no file of their own. A second file under projects/zeppelin-react records only what differs there, so a rule has one home rather than two copies that drift.
  • Coverage via @vitest/coverage-v8 on both sides. Reported, not gated: with a handful of specs in the tree any threshold set today is either meaningless or permanently red. Coverage sets no include — naming a directory makes the provider parse every source under it, and decorator syntax fails there while ZEPPELIN-6637 is open. Coverage parses through a separate path from the test transform, so widening it after 6637 needs re-checking rather than assuming it follows.
  • Vitest lint rules on every unit spec in the package. These catch specs that cannot fail: no assertion, an assertion inside a conditional, a duplicate title, a stray it.only. The React remote gets them too; its lint runs on every pull request through lint:react — inside the run-playwright-e2e-tests job, since frontend.yml builds this module with -DskipTests — so they take effect there immediately even though its test suite does not run in CI yet.
  • test:shell now also covers projects/zeppelin-sdk and projects/zeppelin-visualization. Neither needs a runner of its own, but until now a spec added there would have sat unrun, and the lint rules would not have reached it either.
  • e2e/AGENTS.md gains one corrected line: it is no longer a companion to the repository root, but to this package file.
  • vite is now pinned in both package.json files. Neither declared it, so vitest's transitive resolution had drifted — 8.2.1 in the shell, 8.0.16 in the remote — and a config warning fired in one but not the other. Aligning the remote surfaces that warning, so vitest.config.ts becomes .mts, matching the shell config. 36 tests pass on either version and coverage is unchanged; this is version hygiene, not a fix. It rides along because this change already touches both package.json files and both lock files. Happy to split it out.

One thing the exercise turned up: get-keyword-positions and the line-map module it builds on have no callers. The spec is kept as a worked example and says so at the top; whether to keep or remove the code is left to a follow-up.

Scope is deliberately limited to code that can be tested today. TestBed-based component specs need ZEPPELIN-6637, and wiring the React suite into regular pull-request CI is ZEPPELIN-6566.

What type of PR is it?

Improvement

Todos

  • AGENTS.md for the package and for the React remote
  • Coverage reporting on both sides, no thresholds
  • Vitest lint rules on both sides
  • Example specs for get-keyword-positions, HumanizeBytesPipe and textUtils
  • Reach projects/zeppelin-sdk and projects/zeppelin-visualization from the runner, the spec tsconfig and lint
  • Pin vite in both packages and rename the remote's vitest config to .mts

What is the Jira issue?

ZEPPELIN-6648

How should this be tested?

cd zeppelin-web-angular
npm run test:shell                 # 19 tests
npm run test:shell -- --coverage   # report under coverage/, no gate
npm run lint                       # includes lint:react

cd projects/zeppelin-react
npm test                           # 36 tests

To confirm the lint rules bite, add a spec with no assertion to either package and run the matching lint command; it is reported as an error.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No, the added AGENTS.md files are the documentation

@voidmatcha
voidmatcha force-pushed the unit-test-conventions branch 5 times, most recently from 3ffc8bc to f105909 Compare August 23, 2026 02:39
@voidmatcha
voidmatcha force-pushed the unit-test-conventions branch from f105909 to d9b9f3b Compare August 23, 2026 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant