Skip to content

Conversation

@jsmid1
Copy link
Collaborator

@jsmid1 jsmid1 commented Nov 12, 2025

This PR implements automation for Nexus plugin UI checks.

Issue: RHTAP-5669

Summary by CodeRabbit

  • New Features

    • Added Nexus registry UI support with repository heading and table header verification.
  • UI

    • Registry tables now include additional column headers (Version, Artifact, Repository Type, Checksum, Modified, Size) across supported providers.
  • Tests

    • UI tests updated to hide the Quick Start side panel during CI and Image Registry flows for more stable verification.

@jsmid1 jsmid1 requested a review from xinredhat as a code owner November 12, 2025 08:53
@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

Walkthrough

Adds Nexus registry UI test support: new Nexus page-object constants, a NexusUiPlugin with table-header and heading checks, refactors BaseRegistryPlugin to make table-header checks abstract, updates Quay plugin to implement the abstract method, and adds Quick Start dismissal steps in tests.

Changes

Cohort / File(s) Summary
Page Object Enhancements
src/ui/page-objects/registryPo.ts
Added Nexus-related labels and column-header constants: versionColumnHeader, artifactColumnHeader, repositoryTypeColumnHeader, checksumColumnHeader, modifiedColumnHeader, and nexusRepositoryPrefix.
Registry Plugin API / Base
src/ui/plugins/registry/baseRegistryPlugin.ts, src/ui/plugins/registry/registryPlugin.ts
Made checkTableColumnHeaders(page) abstract in BaseRegistryPlugin and removed the checkVulnerabilities(page) method from the public interface.
Nexus Registry Implementation
src/ui/plugins/registry/nexusUiPlugin.ts, src/ui/plugins/registry/registryUiFactory.ts
Added NexusUiPlugin (constructor, checkRepositoryHeading, stubbed checkRepositoryLink, checkTableColumnHeaders) and wired factory to return it for ImageRegistryType.NEXUS.
Quay Plugin Updates
src/ui/plugins/registry/quayUiPlugin.ts
Removed internal quayProvider field usage; updated to use this.registry and implemented checkTableColumnHeaders(page) verifying Quay-specific headers via RegistryPO.
Test Flow Updates
tests/ui/component.test.ts
Inserted hideQuickStartIfVisible(page) steps after navigating to CI tab and Image Registry page to dismiss Quick Start during tests.

Sequence Diagram

sequenceDiagram
    participant Test as Test Flow
    participant Factory as RegistryUIFactory
    participant Plugin as RegistryUiPlugin
    participant PO as RegistryPO/Page

    Test->>Factory: createRegistryUiPlugin(registry)
    Factory-->>Plugin: NexusUiPlugin | QuayUiPlugin | ...
    Note over Test,Plugin: Repository heading check
    Test->>Plugin: checkRepositoryHeading(page)
    Plugin->>PO: locate heading using registry prefix
    PO-->>Plugin: heading element
    Plugin->>Plugin: assert visible
    Note over Test,Plugin: Table headers check
    Test->>Plugin: checkTableColumnHeaders(page)
    Plugin->>PO: locate header cells (provider-specific set)
    PO-->>Plugin: header elements
    Plugin->>Plugin: assert all visible
    Plugin-->>Test: checks complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify BaseRegistryPlugin interface change: ensure all implementations (Quay, Nexus, any others) implement the new abstract checkTableColumnHeaders.
  • Confirm RegistryPO header strings match the actual UI text/selectors used by Nexus and Quay.
  • Inspect NexusUiPlugin checkRepositoryLink() stub to determine expected future behavior and ensure tests don't rely on it.
  • Check test timing around hideQuickStartIfVisible() calls to avoid flaky CI races.

Possibly related PRs

  • PR #85: Prior changes to registry UI abstractions (RegistryPO, Base/Quay plugin structure) that this PR extends — likely touches same files and interfaces.
  • PR #125: Introduced/exported hideQuickStartIfVisible() helper used by the new test steps in this PR.

Suggested reviewers

  • xinredhat
  • Katka92

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: implementing checks for the Nexus plugin, which aligns with the core additions of NexusUiPlugin class and related Nexus support throughout the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added enhancement New feature or request testing ui-testing labels Nov 12, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/ui/plugins/registry/quayUiPlugin.ts (1)

33-61: Remove orphaned checkVulnerabilities method from QuayUiPlugin (lines 33–61).

The method is dead code—never called in active test flows (the test reference at lines 173–175 is explicitly commented out), not part of the abstract contract in BaseRegistryPlugin, and absent from other registry plugins like NexusUiPlugin. The commented note indicates the security scan timing dependency is temporary, but without an active test requirement, this implementation should be removed to maintain a clean, maintainable test interface aligned with the updated contract pattern (e.g., checkTableColumnHeaders).

🧹 Nitpick comments (2)
tests/ui/component.test.ts (1)

148-150: Consider adding waitForPageLoad before hiding the Quick Start panel.

The CI (lines 78-83) and Docs (lines 104-113) test flows call waitForPageLoad after navigation and before hiding the Quick Start panel, but this Image Registry test skips that step. While hideQuickStartIfVisible has its own wait logic, maintaining consistency across test flows improves predictability and reduces flaky test risk.

Apply this diff to align with the established pattern:

       await page.goto(`${component.getComponentUrl()}/image-registry`, {
         timeout: 20000,
       });
+      await waitForPageLoad(page, component.getCoreComponent().getName());
 
       await test.step('Hide Quick start side panel', async () => {
         await hideQuickStartIfVisible(page);
       }, { timeout: 20000 });
src/ui/plugins/registry/nexusUiPlugin.ts (1)

18-21: Document the no-op method more explicitly.

Since Nexus repository links are not yet supported, consider adding a TODO comment or issue reference to track this missing functionality for future implementation.

     // eslint-disable-next-line no-unused-vars
     async checkRepositoryLink(_page: Page): Promise<void> {
-        // Skipped: Nexus repository link is not supported yet
+        // TODO: Implement Nexus repository link verification once supported
+        // Skipping this check for now as the Nexus UI plugin does not yet expose repository links
     }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c30183 and 7f5232e.

📒 Files selected for processing (7)
  • src/ui/page-objects/registryPo.ts (2 hunks)
  • src/ui/plugins/registry/baseRegistryPlugin.ts (1 hunks)
  • src/ui/plugins/registry/nexusUiPlugin.ts (1 hunks)
  • src/ui/plugins/registry/quayUiPlugin.ts (1 hunks)
  • src/ui/plugins/registry/registryPlugin.ts (0 hunks)
  • src/ui/plugins/registry/registryUiFactory.ts (2 hunks)
  • tests/ui/component.test.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • src/ui/plugins/registry/registryPlugin.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/ui/**

⚙️ CodeRabbit configuration file

Focus on page object pattern implementation, element selection strategies, UI test stability, maintainability, proper wait conditions, error handling, and flaky test prevention. Ensure tests are resilient to UI changes.

Files:

  • src/ui/plugins/registry/baseRegistryPlugin.ts
  • src/ui/page-objects/registryPo.ts
  • src/ui/plugins/registry/registryUiFactory.ts
  • src/ui/plugins/registry/quayUiPlugin.ts
  • src/ui/plugins/registry/nexusUiPlugin.ts
tests/**

⚙️ CodeRabbit configuration file

Focus on test coverage, maintainability, proper test structure, test isolation, mock usage, async test patterns, and test stability. Ensure tests are deterministic and properly clean up after themselves.

Files:

  • tests/ui/component.test.ts
**/*.test.ts

⚙️ CodeRabbit configuration file

Ensure comprehensive test coverage, proper assertions, test isolation, async pattern correctness, mock usage, and test maintainability. Review for potential flaky tests and race conditions.

Files:

  • tests/ui/component.test.ts
🧠 Learnings (1)
📚 Learning: 2025-07-31T07:43:28.355Z
Learnt from: rhopp
Repo: redhat-appstudio/tssc-test PR: 72
File: package.json:15-15
Timestamp: 2025-07-31T07:43:28.355Z
Learning: In the tssc-test project, the `test:ui` script intentionally does not include the `generate-config` step. This is expected behavior because UI tests are designed to run in scenarios where the project-configs.json file already exists - either from prior E2E test execution or from manual template creation in RHDH.

Applied to files:

  • tests/ui/component.test.ts
🧬 Code graph analysis (4)
src/ui/plugins/registry/registryUiFactory.ts (1)
src/ui/plugins/registry/nexusUiPlugin.ts (1)
  • NexusUiPlugin (6-31)
src/ui/plugins/registry/quayUiPlugin.ts (1)
src/ui/page-objects/registryPo.ts (1)
  • RegistryPO (8-39)
src/ui/plugins/registry/nexusUiPlugin.ts (2)
src/rhtap/core/integration/registry/imageRegistry.ts (1)
  • ImageRegistry (9-28)
src/ui/page-objects/registryPo.ts (1)
  • RegistryPO (8-39)
tests/ui/component.test.ts (1)
src/ui/commonUi.ts (1)
  • hideQuickStartIfVisible (22-39)
🔇 Additional comments (6)
src/ui/page-objects/registryPo.ts (2)

17-22: LGTM!

The Nexus column header constants follow the established naming convention and align with the existing Quay header structure.


37-37: LGTM!

The Nexus repository prefix constant is consistent with the existing quayRepositoryPrefix pattern.

src/ui/plugins/registry/registryUiFactory.ts (1)

11-11: LGTM!

The Nexus integration follows the established factory pattern and maintains consistency with the existing QUAY implementation.

Also applies to: 29-30

src/ui/plugins/registry/quayUiPlugin.ts (1)

24-31: LGTM!

The table header validation correctly implements the abstract method from BaseRegistryPlugin and uses appropriate role-based selectors with RegistryPO constants for all six Quay-specific column headers.

src/ui/plugins/registry/baseRegistryPlugin.ts (1)

65-65: LGTM!

Replacing checkVulnerabilities with checkTableColumnHeaders as the abstract method aligns the base interface with the new testing approach and is consistently implemented by both Quay and Nexus plugins.

src/ui/plugins/registry/nexusUiPlugin.ts (1)

23-30: LGTM!

The table column header validation correctly verifies all six Nexus-specific headers using RegistryPO constants and role-based selectors. The implementation aligns with the abstract method contract from BaseRegistryPlugin.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/ui/plugins/registry/quayUiPlugin.ts (1)

22-29: Consider using Promise.all for parallel assertions.

While the sequential await pattern works, running all six column header assertions in parallel would improve test performance and reduce potential timeout issues.

 async checkTableColumnHeaders(page: Page): Promise<void> {
-    await expect(page.getByRole('columnheader', { name: RegistryPO.tagColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.lastModifiedColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.securityScanColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.sizeColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.expiresColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.manifestColumnHeader })).toBeVisible();
+    await Promise.all([
+        expect(page.getByRole('columnheader', { name: RegistryPO.tagColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.lastModifiedColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.securityScanColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.sizeColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.expiresColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.manifestColumnHeader })).toBeVisible()
+    ]);
 }
src/ui/plugins/registry/nexusUiPlugin.ts (2)

16-19: Track the stubbed repository link check.

The method is appropriately stubbed with a clear explanation, but this functionality gap should be tracked to ensure it's implemented when Nexus repository links are supported.

Would you like me to open an issue to track the implementation of checkRepositoryLink for Nexus?


21-28: Consider using Promise.all for parallel assertions.

Similar to the Quay plugin, running all six column header assertions in parallel would improve test performance and resilience.

 async checkTableColumnHeaders(page: Page): Promise<void> {
-    await expect(page.getByRole('columnheader', { name: RegistryPO.versionColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.artifactColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.repositoryTypeColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.checksumColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.modifiedColumnHeader })).toBeVisible();
-    await expect(page.getByRole('columnheader', { name: RegistryPO.sizeColumnHeader })).toBeVisible();
+    await Promise.all([
+        expect(page.getByRole('columnheader', { name: RegistryPO.versionColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.artifactColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.repositoryTypeColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.checksumColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.modifiedColumnHeader })).toBeVisible(),
+        expect(page.getByRole('columnheader', { name: RegistryPO.sizeColumnHeader })).toBeVisible()
+    ]);
 }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78d2ff9 and 4106eb8.

📒 Files selected for processing (7)
  • src/ui/page-objects/registryPo.ts (2 hunks)
  • src/ui/plugins/registry/baseRegistryPlugin.ts (1 hunks)
  • src/ui/plugins/registry/nexusUiPlugin.ts (1 hunks)
  • src/ui/plugins/registry/quayUiPlugin.ts (2 hunks)
  • src/ui/plugins/registry/registryPlugin.ts (0 hunks)
  • src/ui/plugins/registry/registryUiFactory.ts (2 hunks)
  • tests/ui/component.test.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • src/ui/plugins/registry/registryPlugin.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/ui/page-objects/registryPo.ts
  • tests/ui/component.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/ui/**

⚙️ CodeRabbit configuration file

Focus on page object pattern implementation, element selection strategies, UI test stability, maintainability, proper wait conditions, error handling, and flaky test prevention. Ensure tests are resilient to UI changes.

Files:

  • src/ui/plugins/registry/registryUiFactory.ts
  • src/ui/plugins/registry/quayUiPlugin.ts
  • src/ui/plugins/registry/baseRegistryPlugin.ts
  • src/ui/plugins/registry/nexusUiPlugin.ts
🧬 Code graph analysis (3)
src/ui/plugins/registry/registryUiFactory.ts (1)
src/ui/plugins/registry/nexusUiPlugin.ts (1)
  • NexusUiPlugin (6-29)
src/ui/plugins/registry/quayUiPlugin.ts (1)
src/ui/page-objects/registryPo.ts (1)
  • RegistryPO (8-39)
src/ui/plugins/registry/nexusUiPlugin.ts (2)
src/rhtap/core/integration/registry/imageRegistry.ts (1)
  • ImageRegistry (9-28)
src/ui/page-objects/registryPo.ts (1)
  • RegistryPO (8-39)
🔇 Additional comments (5)
src/ui/plugins/registry/baseRegistryPlugin.ts (1)

65-65: LGTM! Well-designed abstraction.

The refactoring to make checkTableColumnHeaders abstract allows each registry plugin to define its own table structure while maintaining shared functionality in the base class. Both Quay and Nexus implementations properly implement this method.

src/ui/plugins/registry/registryUiFactory.ts (1)

11-11: LGTM! Consistent factory pattern.

The Nexus plugin integration follows the established factory pattern consistently with the existing Quay implementation.

Also applies to: 29-30

src/ui/plugins/registry/quayUiPlugin.ts (1)

13-13: Good refactoring to use inherited registry field.

The change from quayProvider to this.registry properly leverages the inherited field from BaseRegistryPlugin, eliminating duplication and improving maintainability.

Also applies to: 17-17, 57-57

src/ui/plugins/registry/nexusUiPlugin.ts (2)

6-10: LGTM! Clean class structure.

The constructor properly delegates to the base class and relies on the inherited registry field without duplication.


12-14: LGTM! Proper use of page object constants.

The repository heading check correctly uses the RegistryPO constants and inherited registry accessors.

@konflux-ci-qe-bot
Copy link

@jsmid1: The following test has Failed, say /retest to rerun failed tests.

PipelineRun Name Status Rerun command Build Log Test Log
e2e-4.19-jk57w Failed /retest View Pipeline Log View Test Logs

Inspecting Test Artifacts

To inspect your test artifacts, follow these steps:

  1. Install ORAS (see the ORAS installation guide).
  2. Download artifacts with the following commands:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/rhtap-team/rhtap-cli:e2e-4.19-jk57w

Test results analysis

<not enabled>

OCI Artifact Browser URL

<not enabled>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants