Skip to content

[ZEPPELIN-6660] Add notebook parity registry - #5456

Open
voidmatcha wants to merge 1 commit into
apache:masterfrom
voidmatcha:ZEPPELIN-6660-parity-scenarios
Open

[ZEPPELIN-6660] Add notebook parity registry#5456
voidmatcha wants to merge 1 commit into
apache:masterfrom
voidmatcha:ZEPPELIN-6660-parity-scenarios

Conversation

@voidmatcha

Copy link
Copy Markdown
Member

What is this PR for?

This PR records the current Angular Notebook behaviour in a reusable parity registry.

The editable source is JSON and the human-readable Markdown table is generated from it. Each scenario has a stable NB-PARITY-### identifier and links either to an existing Playwright E2E test or to the Jira issue that tracks the remaining work.

The validator rejects drift between JSON and generated Markdown, duplicate or invalid scenario identifiers, unsupported coverage states, a covered claim without an executable Playwright test carrying the same identifier, incomplete scenarios without a Jira issue, and invalid evidence or test paths.

Existing Notebook Playwright test titles now carry the stable scenario identifiers, and the frontend CI workflow runs the registry check.

This does not implement missing E2E behaviour or claim that React Notebook parity is complete. It establishes the characterization baseline that later Angular, Shared Core, and React work can compare against.

The generated registry and validation approach follows Apache Ambari's React parity matrix work:

What type of PR is it?

Improvement

Todos

  • Add a JSON Notebook parity registry
  • Generate the reviewable Markdown view from the registry
  • Add generation and validation tests, including failure cases
  • Link existing Notebook Playwright coverage through stable scenario identifiers
  • Run the registry validation in frontend CI

What is the Jira issue?

ZEPPELIN-6660

How should this be tested?

cd zeppelin-web-angular
npm run check:notebook-parity-scenarios

The command passes after rebasing on the latest origin/master: 15 validator tests pass and the checked-in registry matches its generated Markdown view.

The quick GitHub Actions workflow has passed. The frontend and core workflows are still running for the rebased commit.

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? Yes. notebook-parity.md is the generated review document for the registry.

@tbonelee

tbonelee commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

A few suggestions.

1. Move the schema definition to a declarative form

validateRegistry is effectively the schema, and most of it (field types, enums, patterns, the per-status requirements) can be declared instead. Moving that to a JSON Schema file and validating with Ajv would leave only the rules that have to look at the outside world: path existence, git commit reachability, and the test() declaration check.

Adding $schema to the data file also surfaces errors in the editor while a contributor is editing, rather than only in CI. The status branching maps to if/then, or to discriminatedUnion if you prefer zod. With a schema file, schemaVersion becomes redundant with $schema.

2. Playwright tags instead of test titles

Putting the identifier in the title and matching it as a substring means routine title cleanups break the registry. Playwright here is 1.55.1, so tags are available:

test('should display notebook container with proper structure', { tag: '@NB-PARITY-001' }, async () => {});

3. Does the role axis need to be required on every scenario?

NB-PARITY-001 and 060 are allow for every role, so there is no permission dimension there. And 003 (entering edit mode) marks reader as deny while 010 and 011, which happen inside the same editor, mark it n/a, so "not relevant" and "should be denied" are mixed together. Would making the field optional, or requiring it only for area: "permission" scenarios, work better?

roleVerification also allows only unverified and not-applicable, so there is no value to record a permission test once one is written.

4. Duplicated strings that are not identifiers

Referencing scenarios by id is the right call, but the same principle is not applied one level down, so several values are copied verbatim.

  • uncoveredOutcomes repeats whole sentences from observableOutcomes. Could the outcomes carry ids and be referenced instead?
  • evidence points at tests on covered rows and at production sources on gap rows. 060 is a gap yet lists an existing spec as evidence, which makes it hard to tell whether that test covers the scenario. Splitting "where it is implemented" from "what demonstrates it" would let each side carry its own rule.
  • projects is a hand copy of test.skip(browserName !== 'chromium', …) in the spec, and it is not checked. Removing that skip leaves the registry stale and CI green.

5. Consistency with e2e/AGENTS.md

e2e/AGENTS.md:124 currently says "no cross-framework parity project in this config… do not build parity infrastructure ahead of need." If this PR is the decision to change that stance, could that paragraph be updated in the same PR? The convention that test titles must keep the identifier is not documented anywhere either, so a contributor following that file would not know why CI broke.

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.

2 participants