[RHTAP-5669] Implements checks for Nexus plugin #221
Conversation
WalkthroughAdds Nexus UI support and registry header constants: introduces NexusUiPlugin, makes table-header checks abstract in BaseRegistryPlugin, updates Quay implementation to the new API, extends RegistryPO with Nexus column/label constants, and adds Quick Start dismissal steps in tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as E2E Test
participant Factory as RegistryUiFactory
participant Plugin as RegistryUiPlugin (Quay/Nexus)
participant PO as RegistryPO
Test->>Factory: createRegistryUiPlugin(registry)
Factory-->>Plugin: returns provider-specific Plugin
Note right of Plugin: Provider-specific checks
Test->>Plugin: checkRepositoryHeading(page)
Plugin->>PO: query heading using prefix + registry identifiers
PO-->>Plugin: heading element
Plugin-->>Test: assert visible
Test->>Plugin: checkTableColumnHeaders(page)
Plugin->>PO: query provider-specific header labels
PO-->>Plugin: header elements
Plugin-->>Test: assert all headers visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (3)tests/**⚙️ CodeRabbit configuration file
Files:
**/*.test.ts⚙️ CodeRabbit configuration file
Files:
src/ui/**⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (3)tests/ui/component.test.ts (1)
src/ui/plugins/registry/quayUiPlugin.ts (1)
src/ui/plugins/registry/nexusUiPlugin.ts (2)
🔇 Additional comments (9)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 orphanedcheckVulnerabilitiesmethod fromQuayUiPlugin(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 likeNexusUiPlugin. 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
waitForPageLoadafter navigation and before hiding the Quick Start panel, but this Image Registry test skips that step. WhilehideQuickStartIfVisiblehas 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
📒 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.tssrc/ui/page-objects/registryPo.tssrc/ui/plugins/registry/registryUiFactory.tssrc/ui/plugins/registry/quayUiPlugin.tssrc/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
quayRepositoryPrefixpattern.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
BaseRegistryPluginand 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
checkVulnerabilitieswithcheckTableColumnHeadersas 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.
jsmid1
force-pushed
the
RHTAP-5669
branch
2 times, most recently
from
78d2ff9 to
4106eb8
Compare
There was a problem hiding this comment.
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
checkRepositoryLinkfor 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
📒 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.tssrc/ui/plugins/registry/quayUiPlugin.tssrc/ui/plugins/registry/baseRegistryPlugin.tssrc/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
checkTableColumnHeadersabstract 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
quayProvidertothis.registryproperly leverages the inherited field fromBaseRegistryPlugin, 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
registryfield without duplication.
12-14: LGTM! Proper use of page object constants.The repository heading check correctly uses the
RegistryPOconstants and inherited registry accessors.
|
@jsmid1: The following test has Failed, say /retest to rerun failed tests.
Inspecting Test ArtifactsTo inspect your test artifacts, follow these steps:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/rhtap-team/rhtap-cli:e2e-4.19-ws2wlTest results analysis<not enabled> OCI Artifact Browser URL<not enabled> |
2 participants
This PR implements automation for Nexus plugin UI checks.
Issue: RHTAP-5669
Summary by CodeRabbit
New Features
UI
Tests