-
Notifications
You must be signed in to change notification settings - Fork 10
[RHTAP-5669] Implements checks for Nexus plugin #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds 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
Sequence DiagramsequenceDiagram
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
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)
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 |
There was a problem hiding this 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 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.
There was a problem hiding this 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
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-jk57wTest results analysis<not enabled> OCI Artifact Browser URL<not enabled> |
This PR implements automation for Nexus plugin UI checks.
Issue: RHTAP-5669
Summary by CodeRabbit
New Features
UI
Tests