Skip to content

chore(ui): Revamp test suite details page#25139

Merged
shah-harshit merged 15 commits intomainfrom
issue-23398
Feb 19, 2026
Merged

chore(ui): Revamp test suite details page#25139
shah-harshit merged 15 commits intomainfrom
issue-23398

Conversation

@shah-harshit
Copy link
Copy Markdown
Contributor

@shah-harshit shah-harshit commented Jan 8, 2026

Describe your changes:

Fixes #23398

Screenshot 2026-01-08 at 3 37 04 PM

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • MUI migration:
    • Replaced Ant Design components (Row, Col, Tabs, Divider, Modal) with Material-UI v7 equivalents (Box, Stack, Tabs, Divider, Dialog)
    • Removed custom LESS stylesheet in favor of MUI theme-based styling with sx prop
  • UI layout restructure:
    • Moved description component from page level into both Test Cases and Pipeline tabs with wrapInCard styling
    • Extracted renderDescription() function to eliminate duplication across tabs
    • Restructured tabs object to use named properties (testCasesTab, pipelineTab) instead of array
  • Styling with MUI theme:
    • Applied theme-based borders, border-radius, and background colors using theme.palette and theme.spacing
    • Replaced custom CSS classes with inline sx prop styling for consistent theming
  • Code quality improvements:
    • Wrapped onDescriptionUpdate in useCallback with proper dependencies (testSuite, t)
    • Corrected React hook dependency arrays by removing unused testOwners references
    • Added activeTab state management for MUI Tabs component

This will update automatically on new commits.


@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 8, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.72% (56343/85732) 45.13% (29494/65357) 47.95% (8895/18552)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR revamps the test suite details page UI by restructuring the layout to improve consistency and visual organization. The description component is moved from page level into both the "Test Cases" and "Pipeline" tabs, metadata display is enhanced with a bordered container, and React hook dependencies are corrected.

Key Changes:

  • Moved description component into tab content with card-style wrapping for consistent presentation
  • Enhanced metadata header with bordered container styling and improved owner/domain labels
  • Fixed React hook dependency arrays by removing stale testOwners reference and wrapping onDescriptionUpdate in useCallback

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
TestSuiteDetailsPage.component.tsx Restructured component layout to move description into tabs, wrapped onDescriptionUpdate in useCallback, fixed dependency arrays for onUpdateOwner and handleDomainUpdate, and added responsive Grid layout with gutter spacing
test-suite-details-page.styles.less Removed unused .test-suite-description styles, added .test-suite-header-metadata for bordered metadata container, added .test-suite-details-tabs for tab overflow handling, and added .test-suite-content-container for tab content styling with borders and padding

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Feb 18, 2026

Code Review 👍 Approved with suggestions 9 resolved / 10 findings

Clean MUI migration with one prior minor finding still unresolved (missing E2E test cleanup for table data). The waitForResponse wildcard bug has been properly fixed. Component code is well-structured with proper memoization.

💡 Quality: E2E test cleanup removed — test data leaked after run

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts:39

The afterAll('Clean up', ...) block that deleted table1 and table2 was removed from TestSuitePipelineRedeploy.spec.ts, but no alternative cleanup mechanism was added. The test creates two tables with test suites and pipelines in beforeAll but never cleans them up.

Most other E2E tests in this project (69 occurrences across 51 files) follow the pattern of cleaning up created resources in afterAll. Leaking test entities can cause:

  • Test environment pollution over time
  • Potential flaky tests if entity names collide
  • Increased load on CI test databases
Suggested fix
  test.beforeEach('Visit home page', async ({ page }) => {
    await redirectToHomePage(page);
  });

  test.afterAll('Clean up', async ({ browser }) => {
    const { afterAction, apiContext } = await createNewPage(browser);

    await table1.delete(apiContext);
    await table2.delete(apiContext);

    await afterAction();
  });
✅ 9 resolved
Bug: waitForResponse uses literal * in includes() — never matches

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts:65
The refactored waitForResponse callback uses response.url().includes('/api/v1/services/ingestionPipelines/deploy/*') which checks for a literal asterisk in the URL. String.includes() does not support glob patterns.

The actual deploy URL will be /api/v1/services/ingestionPipelines/deploy/{uuid}, which does not contain a literal *. This means the predicate never returns true, causing redeployResponse to never resolve and the test to hang until timeout.

The previous code page.waitForResponse('/api/v1/services/ingestionPipelines/deploy/*') worked because Playwright treats string arguments to waitForResponse as URL glob patterns where * is a wildcard.

Fix: Use a regex or a proper substring match (without the trailing /*) inside the callback:

const redeployResponse = page.waitForResponse(
  (response) =>
    response.url().includes('/api/v1/services/ingestionPipelines/deploy/') &&
    response.request().method() === 'POST' &&
    response.status() === 200
);
Quality: Dead CSS selector targets unused MuiTabPanel-root

📄 openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsx:692
The sx prop on line 692 includes a CSS rule targeting '& .MuiTabPanel-root', but no TabPanel component is used in this file. Tab content is rendered manually via conditional {activeTab === ... && tabs.*.children} on lines 696-697. This means the CSS rule is dead code that never matches any element.

This likely indicates a leftover from when MUI's TabPanel component was considered but then a manual tab-switching approach was chosen instead.

Bug: Missing entityName prop in renderDescription()

📄 openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsx:452-459
The old DescriptionV1 usage included entityName={getEntityName(testSuite)}, but the new renderDescription() function omits it. While entityName is optional, it's used for the edit description modal header ("Edit description for {entityName}"). Dropping it means the header will render as "Edit description for " with a blank entity name, which is a minor UX regression.

Add the prop back to maintain the previous behavior.

Quality: Inconsistent spacing/styling between testCasesTab and pipelineTab

📄 openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsx:474-481 📄 openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsx:511-518
The two tab content wrappers have different styling values that appear unintentional:

  • testCasesTab (line 474): spacing={4}, borderRadius: 1.25, p: 4
  • pipelineTab (line 511): spacing={2.5}, borderRadius: 1, p: 2

These inconsistencies create visual differences between tabs (different internal padding, different corner radius, different spacing between description and content). Both tabs serve a similar purpose (wrapping description + content), so they should use the same container styling for visual consistency. Consider extracting a shared sx object or aligning the values.

Quality: Duplicate padding: className="p-md" conflicts with sx p prop

📄 openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsx:474-476
Both Stack components in the tab content (test cases tab and pipeline tab) apply padding twice:

  1. className="p-md" — applies padding via CSS utility class
  2. sx={{ p: 4 }} (or p: 2) — applies padding via MUI's sx prop

These will conflict and create unpredictable styling results depending on CSS specificity. Since the PR is migrating away from CSS utility classes to MUI theming, the className="p-md" should be removed.

Fix: Remove className="p-md" from both Stack components and rely solely on the sx={{ p: ... }} prop.

...and 4 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

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.

Revamp Test Suite details page

3 participants