Fix two rendering defects in the Test Connection modal by Khairajani · Pull Request #31639 · open-metadata/OpenMetadata

Khairajani · 2026-08-17T13:14:55Z

Describe your changes:

Fixes #

Two rendering defects in the Test Connection modal. Both are independent of which connector produced the result, and hold for both the legacy test_connection_db_common payload and the newer @check-framework payload.

1. A failed step's summary line was painted in the success colour.

Both the capability accordion and the raw connection log rendered resultSummary || result.message with a hardcoded tw:text-utility-success-300:

{(result.resultSummary || result.message) &&
  renderColoredLines(`  ${...}`, 'tw:text-utility-success-300', 'sum-')}

Per the schema, resultSummary is "Human-readable summary of what the step found on success", while message on a failed step is the test connection definition's errorMessage. So a connector reporting "Failed to connect to X, please validate the credentials" had that line rendered green, reading as though the step had passed. The colour is now taken from result.passed; a failed step's line gets the advisory colour, which also distinguishes it from the raw errorLog immediately below.

2. The remediation card's raw-error block was uncapped.

A driver that dumps a full stack trace on failure — teradatasql emits ~25 lines of gosqldriver frames, JDBC connectors are similar — grew the card past the height of the modal, pushing the capability checks and the footer actions out of view. It now scrolls at max-h-[220px], matching the cap the raw log section already had at 360px. Nothing is truncated; the block scrolls.

Type of change:

Bug fix

High-level design:

N/A — small change.

Tests:

Use cases covered

A failed step's message is not rendered as though the step succeeded.
A passed step's summary keeps the success colour (no regression from the fix).
A failure whose driver emits a long stack trace leaves the capability checks and footer actions reachable.

Unit tests

Added to openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.test.tsx — 47 pass.
- should not render a failed step message in the success colour
- should keep a passed step summary in the success colour
Verified as real guards: reverting only TestConnectionModalUtils.tsx makes the first fail.

Backend integration tests

Not applicable (no backend changes).

Ingestion integration tests

Not applicable (no ingestion changes).

Playwright (UI) tests

Added to openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/TestConnectionModal.spec.ts.
- a long stack trace scrolls inside the remediation card instead of filling the modal — asserts the block's measured height is capped and that it actually scrolls (scrollHeight > clientHeight) rather than being clipped.
The height cap is asserted here rather than in Jest deliberately: jsdom has no layout, so a unit test could only assert the class name is present, not that the element stops growing.

Manual testing performed

Reproduced against a live Teradata 20.0 system. An invalid-credentials test connection returned a ~25-line gosqldriver/teradatasql stack trace that filled the modal, with "Failed to connect to Teradata, please validate the credentials" rendered in green inside the raw log.

Checks run

yarn test src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.test.tsx   # 47 passed
npx tsc --noEmit                  # no errors in the changed files
eslint + prettier (changed files) # 0 errors
yarn token-audit                  # no new violations

UI screen recording / screenshots:

Before/after screenshots to be attached.

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>
My PR is linked to a GitHub issue via Fixes #<issue-number> above.
I have commented on my code, particularly in hard-to-understand areas.
For UI changes: I attached a screen recording and/or screenshots above.
I have added tests (unit + Playwright) and listed them above.
I have added a test that covers the exact scenario we are fixing.

_{Scope note: an earlier revision of this PR also headlined the remediation card with the step's message instead of the raw errorLog. That was dropped. Connectors on the @check framework never populate message — mapper.py fixes it at None — so that path was only reachable from the legacy test_connection_db_common, and would become dead code as connectors migrate. The two changes that remain are true for either payload shape. Teradata's own diagnosis quality is addressed separately in #31642, which is independent of this PR.}

Greptile Summary

This PR corrects two Test Connection modal rendering defects.

Colors failed-step summaries as warnings while retaining success coloring for passed steps.
Caps long remediation error logs in a scrollable region so subsequent checks and footer actions remain reachable.
Adds focused Jest and Playwright regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
openmetadata-ui/src/main/resources/ui/src/utils/TestConnectionModalUtils.tsx	Selects summary colors from step pass state and constrains remediation error logs to a scrollable 220px region.
openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.test.tsx	Adds behavior-level coverage for failed and successful summary colors.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/TestConnectionModal.spec.ts	Verifies that a realistic long stack trace is height-capped and remains internally scrollable.

_{Reviews (5): Last reviewed commit: "Merge branch 'main' into fix/test-connec..." | Re-trigger Greptile}

…ailure card When a test connection step fails, the ingestion framework returns two distinct fields: message - the test connection definition's errorMessage, which is the actionable, connector-authored hint - and errorLog, the raw driver exception. The remediation card preferred errorLog, so a Teradata credential failure filled the card with a Go stack trace while 'Failed to connect to Teradata, please validate the credentials' was only reachable by expanding the raw connection log. Use message as the card headline (diagnosis.title still wins when a connector has an error pack) and keep errorLog below it, capped so a long stack trace no longer dominates the modal. Also stop painting a failed step's message in the success colour in the capability accordion and the raw log - resultSummary is the success channel, message on a failed step is not.

github-actions · 2026-08-17T13:15:05Z

❌ 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 · 2026-08-17T13:15:16Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

github-actions · 2026-08-17T13:16:48Z

✅ Playwright Results — workflow succeeded

Validated commit 79a21ead3a31d76ba7b0f01ec2c84fec922e1751 in Playwright run 32339064703, attempt 1.

✅ 81 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 55m 6s

⏱️ Max setup 5m 1s · max shard execution 11m 57s · max shard-job elapsed before upload 19m 36s · reporting 5s

🌐 162.57 requests/attempt · 1.70 app boots/UI scenario · 0.00% common-shard skew

Optimization targets still in progress:

Application boot ratio was 1.7 per UI scenario (143 boots / 84 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	60	0	0	0	0	0
✅ Shard ingestion-01	21	0	0	0	0	0

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

…ection mock The remediation-card E2E asserted on 'Failed to connect to mysql, please validate the credentials' - real copy from mysql.json - inside a test that mocks the testConnectionDefinitions endpoint outright. Nothing there is coupled to the shipped definition, so the string implied a relationship that does not exist, and it was duplicated between the mock and the assertion where the two could silently drift apart. One named constant each for the message and the error log, with a value that reads as fixture data. What the test actually pins is that whatever the backend returns in message becomes the headline.

github-actions · 2026-08-17T13:43:42Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Scope reduction. The remaining two are straight rendering defects that hold for any payload shape: - A failed step's summary line was hardcoded to the success colour in both the capability accordion and the raw log. resultSummary is the success channel; message on a failed step is the definition's errorMessage, so painting it green renders a failure as a pass. - The remediation card's raw-error block was uncapped, so a driver that dumps a full stack trace pushed the capability checks and the footer actions off the modal. It now scrolls at 220px, matching the raw log section's existing cap. Dropped: headlining the card with the step's message, and the guard and destructuring changes that supported it. The connectors on the new check framework never set message - mapper.py fixes it at None - so that path was only ever reachable from the legacy test_connection_db_common, and would become dead code as connectors migrate. The height cap is asserted in Playwright rather than Jest: jsdom has no layout, so a unit test could only assert the class name, not that the element actually stops growing and scrolls.

github-actions · 2026-08-17T19:44:43Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

github-actions · 2026-08-18T09:51:21Z

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 9 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 9 warning(s) across 2 changed file(s).

Count	Rule
3	`sonarjs/cyclomatic-complexity`
2	`sonarjs/no-duplicate-string`
1	`sonarjs/no-hardcoded-ip`
1	`react/no-array-index-key`
1	`sonarjs/cognitive-complexity`
1	`sonarjs/expression-complexity`

All findings

	Location	Rule	Message
🟡	`src/components/common/TestConnection/TestConnectionModal/TestConnectionModal.test.tsx:190:16`	`sonarjs/no-hardcoded-ip`	Make sure using a hardcoded IP address 10.0.0.1 is safe here.
🟡	`src/utils/TestConnectionModalUtils.tsx:297:51`	`sonarjs/no-duplicate-string`	Define a constant instead of duplicating this literal 3 times.
🟡	`src/utils/TestConnectionModalUtils.tsx:306:53`	`react/no-array-index-key`	Do not use Array index in keys
🟡	`src/utils/TestConnectionModalUtils.tsx:426:17`	`sonarjs/cognitive-complexity`	Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
🟡	`src/utils/TestConnectionModalUtils.tsx:426:17`	`sonarjs/cyclomatic-complexity`	{"message":"Function has a complexity of 19 which is greater than 10 authorized.","cost":9,"secondaryLocations":[{"line":426,"column":16,"endLine":426,"endColum
🟡	`src/utils/TestConnectionModalUtils.tsx:637:15`	`sonarjs/expression-complexity`	Reduce the number of conditional operators (5) used in the expression (maximum allowed 3).
🟡	`src/utils/TestConnectionModalUtils.tsx:696:37`	`sonarjs/cyclomatic-complexity`	{"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":696,"column":36,"endLine":696,"endColum
🟡	`src/utils/TestConnectionModalUtils.tsx:798:27`	`sonarjs/no-duplicate-string`	Define a constant instead of duplicating this literal 3 times.
🟡	`src/utils/TestConnectionModalUtils.tsx:1017:17`	`sonarjs/cyclomatic-complexity`	{"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":1017,"column":16,"endLine":1017,"endCol

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

make ui-checkstyle-changed

github-actions · 2026-08-18T10:00:27Z

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	66.95% (80222/119818)	51.37% (49062/95496)	52.34% (14662/28009)

gitar-bot · 2026-08-20T06:19:09Z

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Fixes the test connection failure card by prioritizing actionable connector error messages over raw stack traces and correcting failure colors. However, the new guard drops remediation cards for diagnosis-only failed steps, and the capability step is still redundantly resolved twice.

⚠️

Bug: Guard drops remediation card for diagnosis-only failed step

📄 openmetadata-ui/src/main/resources/ui/src/utils/TestConnectionModalUtils.tsx:1036-1050

The new guard if (!errorContent) return null only tests errorLog/message and ignores diagnosis. A failed step whose only failure payload is a structured diagnosis (no errorLog, no message) now renders no card at all. This directly contradicts the PR description, which claims the change 'fixes a latent bug ... a step whose only failure payload was a structured diagnosis rendered no card' and that 'the new guard renders it' — the code does the opposite. Include diagnosis in the guard so the card renders whenever any failure payload exists.

Render the card when either errorContent or a structured diagnosis is present.

if (!errorContent && !diagnosis) {
  return null;
}

💡 Quality: Failed capability step is resolved twice via duplicated find

📄 openmetadata-ui/src/main/resources/ui/src/utils/TestConnectionModalUtils.tsx:1036-1050

In the non-gate branch, capabilitySteps.find(... passed === false) plus getConnectionStepResult now runs twice — once to derive errorContent and again to derive diagnosis. This re-introduces the exact duplication the PR description says it removed ('resolves the failed step once (it previously ran the same capabilitySteps.find twice)'). Resolve the failed result once and read both errorContent and diagnosis from it.

Compute the failed step result once, then derive errorContent and diagnosis from it.

const failedResult = connectionFailed
  ? gateResult
  : (() => {
      const failed = capabilitySteps.find(
        (s) => s.mandatory && getConnectionStepResult(s)?.passed === false
      );

      return failed ? getConnectionStepResult(failed) : undefined;
    })();

const errorContent = failedResult?.errorLog || failedResult?.message || '';
const diagnosis = failedResult?.diagnosis;

🤖 Prompt for agents

Code Review: Fixes the test connection failure card by prioritizing actionable connector error messages over raw stack traces and correcting failure colors. However, the new guard drops remediation cards for diagnosis-only failed steps, and the capability step is still redundantly resolved twice.

1. ⚠️ Bug: Guard drops remediation card for diagnosis-only failed step
   Files: openmetadata-ui/src/main/resources/ui/src/utils/TestConnectionModalUtils.tsx:1036-1050

   The new guard `if (!errorContent) return null` only tests `errorLog`/`message` and ignores `diagnosis`. A failed step whose only failure payload is a structured `diagnosis` (no `errorLog`, no `message`) now renders no card at all. This directly contradicts the PR description, which claims the change 'fixes a latent bug ... a step whose only failure payload was a structured diagnosis rendered no card' and that 'the new guard renders it' — the code does the opposite. Include `diagnosis` in the guard so the card renders whenever any failure payload exists.

   Fix (Render the card when either errorContent or a structured diagnosis is present.):
   if (!errorContent && !diagnosis) {
     return null;
   }

2. 💡 Quality: Failed capability step is resolved twice via duplicated find
   Files: openmetadata-ui/src/main/resources/ui/src/utils/TestConnectionModalUtils.tsx:1036-1050

   In the non-gate branch, `capabilitySteps.find(... passed === false)` plus `getConnectionStepResult` now runs twice — once to derive `errorContent` and again to derive `diagnosis`. This re-introduces the exact duplication the PR description says it removed ('resolves the failed step once (it previously ran the same capabilitySteps.find twice)'). Resolve the failed result once and read both `errorContent` and `diagnosis` from it.

   Fix (Compute the failed step result once, then derive errorContent and diagnosis from it.):
   const failedResult = connectionFailed
     ? gateResult
     : (() => {
         const failed = capabilitySteps.find(
           (s) => s.mandatory && getConnectionStepResult(s)?.passed === false
         );
   
         return failed ? getConnectionStepResult(failed) : undefined;
       })();
   
   const errorContent = failedResult?.errorLog || failedResult?.message || '';
   const diagnosis = failedResult?.diagnosis;

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 👍 / 👎 | Powered by Gitar — free for open source}

sonarqubecloud · 2026-08-20T06:33:06Z

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Khairajani requested review from ShaileshParmar11, chirag-madlani and karanh37 as code owners August 17, 2026 13:14

Khairajani mentioned this pull request Aug 17, 2026

Migrate Teradata test connection to the @check framework #31642

Open

13 tasks

Khairajani changed the title ~~Fix: surface the connector error message in the test connection failure card~~ Fix two rendering defects in the Test Connection modal Aug 17, 2026

Khairajani added safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check labels Aug 18, 2026

Merge branch 'main' into fix/test-connection-error-message-surface

09b4b71

Khairajani temporarily deployed to test August 18, 2026 10:15 — with GitHub Actions Inactive

Merge branch 'main' into fix/test-connection-error-message-surface

79a21ea

Khairajani temporarily deployed to test August 20, 2026 06:51 — with GitHub Actions Inactive

Navigation Menu

Uh oh!

Fix two rendering defects in the Test Connection modal - #31639

Fix two rendering defects in the Test Connection modal#31639
Khairajani wants to merge 5 commits into
mainfrom
fix/test-connection-error-message-surface

Khairajani commented Aug 17, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

gitar-bot Bot commented Aug 20, 2026

Uh oh!

sonarqubecloud Bot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Khairajani commented Aug 17, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes:

Type of change:

High-level design:

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

Checks run

UI screen recording / screenshots:

Checklist:

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

github-actions Bot commented Aug 17, 2026

❌ PR checklist incomplete

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Playwright Results — workflow succeeded

Performance

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 9 warning(s)

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jest test Coverage

UI tests summary

Uh oh!

gitar-bot Bot commented Aug 20, 2026

Uh oh!

sonarqubecloud Bot commented Aug 20, 2026

Quality Gate passed for 'open-metadata-ui'

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Khairajani commented Aug 17, 2026 •

edited by greptile-apps Bot

Loading

github-actions Bot commented Aug 17, 2026 •

edited

Loading

github-actions Bot commented Aug 18, 2026 •

edited

Loading