Skip to content

test(e2e): make expandLineageSection wait on state instead of proxies - #31062

Merged
ShaileshParmar11 merged 1 commit into
2.0from
fix/ports-lineage-expand-helper
Aug 5, 2026
Merged

test(e2e): make expandLineageSection wait on state instead of proxies#31062
ShaileshParmar11 merged 1 commit into
2.0from
fix/ports-lineage-expand-helper

Conversation

@ShaileshParmar11

Copy link
Copy Markdown
Contributor

Problem

Two tests in InputOutputPorts.spec.ts have been flaky in 16 of the last 16 AUT 2.0 nightly runs:

  • Section 7: Fullscreen Mode › Toggle fullscreen mode
  • Section 4: Removing Ports › Remove last port shows empty state

Both fail the first attempt on a 60s timeout and pass on retry, so they are permanently in the flaky column and never block a run.

Root cause

expandLineageSection had three defects, all variants of waiting on a proxy rather than the actual state:

  1. Unconditional toggle. The accordion header is a toggle, but the helper clicked it on every call — despite its own doc comment saying "Only expands if currently collapsed". Calling it on an already-expanded section collapses it, and because no /portsView request follows a collapse, the subsequent waitForResponse hangs until the test timeout.

  2. Ambiguous response matcher. url.includes('/portsView') cannot distinguish the lineage fetch from the port-count probe — both hit the same endpoint. The wait could therefore resolve on the counts response while the lineage request was still in flight. The count probe always carries pagination params, so excluding inputLimit= targets the lineage call specifically.

  3. Vacuous loader wait. toHaveCount(0) on [data-testid="loader"] passes when React has not yet mounted the loader, letting callers act on a panel that is still loading. The helper now also waits for a terminal state — ports-lineage-view or .ports-lineage-view-empty.

Regression test

Added Lineage stays expanded when expandLineageSection runs twice. Against the old helper it fails with the same signature as the flaky tests:

Test timeout of 60000ms exceeded.
Error: page.waitForResponse: Target page, context or browser has been closed
   at ../utils/domain.ts:1693

With the fix it passes.

Verification

Check Result
New regression test, before fix failed — 60s timeout, same signature
New regression test, after fix passed
Full InputOutputPorts.spec.ts 44/44 passed
--repeat-each=20 on both flaky tests + new test 60/60 passed

All callers of expandLineageSection live in InputOutputPorts.spec.ts, so the blast radius is one file.

Note on what this proves. Both flaky tests already passed 10/10 locally before this change — the trigger only appears under CI load. Local green therefore demonstrates no regression, not that the CI flake is gone. The idempotency defect (1) is conclusively fixed and covered by a test; confirmation for the flake itself should come from the next few 2.0 nightlies.

Not included

Investigation also surfaced a separate backend issue: /portsView sources data from findToWithOffset + Entity.getEntities(..., NON_DELETED) but total from a separate countFindTo, and silently drops unresolvable records without adjusting total. Soft-deleting an asset that is a port reproduces it deterministically — the UI then shows "Input Ports (1)" beside "assign assets to this Data Product". That is a real user-facing bug but it would not have stabilised these tests, so it belongs in its own change.

🤖 Generated with Claude Code

`Input Output Ports > Section 7 > Toggle fullscreen mode` and
`Section 4 > Remove last port shows empty state` have been flaky in 16 of
the last 16 AUT 2.0 nightly runs, each failing the first attempt on a 60s
timeout and passing on retry.

expandLineageSection had three defects:

1. Unconditional toggle. The accordion header is a toggle, but the helper
   clicked it on every call despite its doc saying "only expands if
   currently collapsed". Calling it on an already-expanded section
   collapsed it, and since no /portsView request follows a collapse the
   subsequent waitForResponse hung until the test timeout.

2. Ambiguous response matcher. `url.includes('/portsView')` cannot
   distinguish the lineage fetch from the port-count probe — both hit the
   same endpoint — so the wait could resolve on the counts response while
   the lineage request was still in flight. The count probe always carries
   pagination params, so excluding `inputLimit=` targets the lineage call.

3. Vacuous loader wait. `toHaveCount(0)` on [data-testid="loader"] passes
   when React has not yet mounted the loader, letting callers act on a
   panel that is still loading. Now also waits for a terminal state:
   ports-lineage-view, or .ports-lineage-view-empty.

Adds a regression test that fails against the old helper with the same
signature as the flaky tests (60s timeout in waitForResponse) and passes
with the fix.

Verified: full InputOutputPorts.spec.ts 44/44; --repeat-each=20 across the
two flaky tests plus the new one, 60/60.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ShaileshParmar11
ShaileshParmar11 requested a review from a team as a code owner August 5, 2026 17:54
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 5, 2026
@ShaileshParmar11
ShaileshParmar11 merged commit 769b7ba into 2.0 Aug 5, 2026
57 of 58 checks passed
@ShaileshParmar11
ShaileshParmar11 deleted the fix/ports-lineage-expand-helper branch August 5, 2026 17:55
@gitar-bot

gitar-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Robustness improvements to the expandLineageSection e2e test helper by adding idempotency checks, precise response matching, and terminal state waits to resolve nightly test flakiness. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

❌ UI Checkstyle Failed

❌ Antd + Less Deprecation Guard

A new antd import or new .less file was added. Use UntitledUI + Tailwind for new work.

Affected files

at Function._resolveFilename (node:internal/modules/cjs/loader:1401:15)
at defaultResolveImpl (node:internal/modules/cjs/loader:1057:19)
at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1062:22)
at Function._load (node:internal/modules/cjs/loader:1211:37)
at TracingChannel.traceSync (node:diagnostics_channel:322:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:171:5)
at node:internal/main/run_main_module:36:49 {


Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

ShaileshParmar11 added a commit that referenced this pull request Aug 6, 2026
…ession test

The regression test added in #31062 asserted that `ports-lineage-view` and
`toggle-fullscreen-btn` were visible after calling expandLineageSection
twice. Both render only when the port data resolves, which is subject to a
CI-only readiness race on the asset association — so the test itself was
flaky in the AUT 2.0 nightlies (both Postgres and MySQL) on its first run:

    Error: expect(locator).toBeVisible() failed
    Locator: getByTestId('ports-lineage-view')
    Error: element(s) not found

The test covers idempotency of the helper, not whether the lineage graph
renders. Assert the accordion's own `aria-expanded` state instead: that is
the actual invariant and it carries no data dependency.

This does not weaken the regression coverage. Against the old helper the
second call collapsed the section and the following waitForResponse hung
until the 60s test timeout, so the test still fails before reaching any
assertion.

Same nightlies confirm the primary fixes landed: "Toggle fullscreen mode"
and "should not show navigation blocker after saving changes" both passed
on the first attempt after being flaky in 16 of the previous 16 runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ShaileshParmar11 added a commit that referenced this pull request Aug 6, 2026
Removes the test added in #31062. Coverage is not lost:
"Section 6: Collapse/Expand Behavior > Lineage section collapse/expand"
already exercises this accordion through the same helper — asserting it
starts collapsed, expands via expandLineageSection, and collapses again.

The only thing the removed test added was calling expandLineageSection
twice in a row. No call site does that: all 15 callers invoke it once,
immediately after navigateToPortsTab on a freshly loaded page where the
section is collapsed. It was guarding a path nothing takes.

It was also a net negative for suite stability. It asserted that
ports-lineage-view and toggle-fullscreen-btn were visible, both of which
render only once port data resolves — so it inherited the CI-only readiness
race on the asset association and was itself flaky in both AUT 2.0
nightlies on its first run:

    Error: expect(locator).toBeVisible() failed
    Locator: getByTestId('ports-lineage-view')
    Error: element(s) not found

The helper's idempotency guard stays in place; it is three self-documenting
lines and does not need an end-to-end test standing behind it. The fix that
mattered is validated directly by the previously flaky tests going green:
"Toggle fullscreen mode" and "should not show navigation blocker after
saving changes" both passed on the first attempt in runs 31060529978 (PG)
and 31060526260 (MySQL), after being flaky in 16 of the previous 16 runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ShaileshParmar11 added a commit that referenced this pull request Aug 6, 2026
`Input Output Ports > Section 7 > Toggle fullscreen mode` and
`Section 4 > Remove last port shows empty state` were flaky in 9 of 9 Main
V2 nightly runs (16/16 on 2.0), failing the first attempt on a 60s timeout
and passing on retry.

expandLineageSection had three defects:

1. Unconditional toggle. The accordion header is a toggle, but the helper
   clicked it on every call despite its doc saying "only expands if
   currently collapsed". Calling it on an already-expanded section
   collapsed it, and since no /portsView request follows a collapse the
   subsequent waitForResponse hung until the test timeout.

2. Ambiguous response matcher. `url.includes('/portsView')` cannot
   distinguish the lineage fetch from the port-count probe — both hit the
   same endpoint — so the wait could resolve on the counts response while
   the lineage request was still in flight. The count probe always carries
   pagination params, so excluding `inputLimit=` targets the lineage call.

3. Vacuous loader wait. `toHaveCount(0)` on [data-testid="loader"] passes
   when React has not yet mounted the loader, letting callers act on a
   panel that is still loading. Now also waits for a terminal state:
   ports-lineage-view, or .ports-lineage-view-empty.

Validated on 2.0 (#31062): both tests passed on the first attempt in the
AUT nightlies 31060529978 (Postgres) and 31060526260 (MySQL).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ShaileshParmar11 added a commit that referenced this pull request Aug 6, 2026
Removes the test added in #31062. Coverage is not lost:
"Section 6: Collapse/Expand Behavior > Lineage section collapse/expand"
already exercises this accordion through the same helper — asserting it
starts collapsed, expands via expandLineageSection, and collapses again.

The only thing the removed test added was calling expandLineageSection
twice in a row. No call site does that: all 15 callers invoke it once,
immediately after navigateToPortsTab on a freshly loaded page where the
section is collapsed. It was guarding a path nothing takes.

It was also a net negative for suite stability. It asserted that
ports-lineage-view and toggle-fullscreen-btn were visible, both of which
render only once port data resolves — so it inherited the CI-only readiness
race on the asset association and was itself flaky in both AUT 2.0
nightlies on its first run:

    Error: expect(locator).toBeVisible() failed
    Locator: getByTestId('ports-lineage-view')
    Error: element(s) not found

The helper's idempotency guard stays in place; it is three self-documenting
lines and does not need an end-to-end test standing behind it. The fix that
mattered is validated directly by the previously flaky tests going green:
"Toggle fullscreen mode" and "should not show navigation blocker after
saving changes" both passed on the first attempt in runs 31060529978 (PG)
and 31060526260 (MySQL), after being flaky in 16 of the previous 16 runs.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant