Skip to content

chore(e2e): run homepage tests with nfs - #3234

Open
debsmita1 wants to merge 5 commits into
redhat-developer:mainfrom
debsmita1:homepage-enable-nfs-e2e
Open

chore(e2e): run homepage tests with nfs#3234
debsmita1 wants to merge 5 commits into
redhat-developer:mainfrom
debsmita1:homepage-enable-nfs-e2e

Conversation

@debsmita1

Copy link
Copy Markdown
Member

@debsmita1
debsmita1 requested review from invincibleJai and removed request for a team August 11, 2026 15:33
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 3:34 PM UTC · Ended 3:36 PM UTC

Commit: e2a947b · View workflow run →

@debsmita1
debsmita1 force-pushed the homepage-enable-nfs-e2e branch from e2a947b to c4d7b0b Compare August 11, 2026 15:35
@debsmita1
debsmita1 requested review from jrichter1, sanketpathak and teknaS47 and removed request for gashcrumb and kadel August 11, 2026 15:35
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:36 PM UTC · Completed 3:42 PM UTC

Commit: c4d7b0b · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [naming-convention] workspaces/homepage/e2e-tests/tests/config/dynamic-plugins-app-next.yaml — The file name dynamic-plugins-app-next.yaml introduces a naming convention with no precedent in the codebase. All sibling workspaces with NFS/app-next support (acr, tech-radar, topology, tekton, analytics, app-defaults) define their app-next project without a separate dynamic-plugins file — none pass a dynamicPlugins option at all. AGENTS.md recommends auto-generation from metadata when no dynamic-plugins.yaml exists.
    Remediation: Follow the established pattern — remove the separate file and rely on auto-generation from metadata, or handle NFS-specific plugin configuration via conditional logic in the spec file as other workspaces do.

Low

  • [consistency] workspaces/homepage/e2e-tests/tests/specs/homepage.spec.ts:49 — The dynamicPlugins option uses WorkspacePaths.resolve() while all 16 other usages across the repo pass bare relative strings without WorkspacePaths.resolve(). Use a bare relative path for consistency: dynamicPlugins: 'tests/config/dynamic-plugins-app-next.yaml'.

  • [consistency] workspaces/homepage/e2e-tests/tests/specs/homepage.spec.ts:20 — The local isNightlyMode() does not check GIT_PR_NUMBER before evaluating E2E_NIGHTLY_MODE. AGENTS.md documents the priority as GIT_PR_NUMBER (forces PR mode) > E2E_NIGHTLY_MODE > JOB_NAME. The intelligent-assistant workspace correctly checks GIT_PR_NUMBER first. Consider adding if (process.env.GIT_PR_NUMBER) return false; at the top of isNightlyMode().

  • [test-integrity] workspaces/homepage/e2e-tests/tests/specs/homepage.spec.ts — The NFS test "NFS: layout persists for same user; clears on account switch" passes clearHomeStorage: true on every reloginAsKeycloakUser call, including the final test1 re-login. This clears localStorage before each login, so the test re-seeds widgets from scratch rather than verifying layout persistence. To actually test persistence, the final test1 re-login should omit clearHomeStorage and verify the previously seeded layout is still present without re-seeding.

Previous run

Review — approve

This PR adds NFS (new frontend system / app-next) support to the homepage E2E tests by introducing a homepage-app-next Playwright project, NFS-specific dynamic plugin configuration, dual-mode test handling, and page object refactoring to accommodate NFS UI differences. The change also adds NFS configuration documentation to the catalog entity and package metadata.

Correctness

The architectural patterns are sound and well-aligned with established conventions:

  • NFS detection via namespace suffix (endsWith("-app-next")) is consistent with at least 6 other workspaces (acr, tech-radar, topology, analytics, tekton, app-defaults) that use the same convention.
  • test.runOnce key split from "homepage-setup" to "homepage-keycloak-groups" + "homepage-deploy-${namespace}" correctly separates cluster-scoped Keycloak setup (shared across projects) from namespace-scoped deployment (unique per project). This follows the orchestrator workspace pattern.
  • isAppNext variable initialized in beforeAll and used in beforeEach/tests is safe under test.describe.serial, which guarantees single-worker serial execution.
  • The custom loginAsKeycloakUser function with a 240s timeout for the Sign In button is a reasonable accommodation for NFS cold-load times, paralleling the global-header workspace's approach.
  • The exitEditMode() branching (dismiss dialog → nudge layout → Save/Cancel fallback) handles the NFS UI's different behavior where Save is only surfaced after a layout dimension change.

Security

No security findings. This is a test-only change with no production code impact. All credentials (homepage-admin@123, test1@123, etc.) are for ephemeral Keycloak test instances in CI, consistent with the existing pattern across 4+ other workspaces. No hidden Unicode, injection patterns, or permission manifest changes were detected.

Style

disabled: false vs enabled: true within the same workspace — The new dynamic-plugins-app-next.yaml uses disabled: false for OCI packages while the existing dynamic-plugins.yaml uses enabled: true for the same package types. Both are functionally equivalent and both conventions are widely used across the repo (older workspaces tend toward enabled: true, newer ones toward disabled: false). This is a minor inconsistency within the homepage workspace but not blocking.

Documentation

The PR updates the catalog entity installation docs (home-page.yaml) and package metadata (red-hat-developer-hub-backstage-plugin-homepage.yaml) with NFS-specific configuration examples. The E2E config version bump from 1.17.0 to 1.17.1 correctly aligns with the metadata. No documentation staleness was introduced by this PR.

Scope

The PR title chore(e2e) slightly understates the scope — it also modifies catalog entity documentation and package metadata, which are not strictly E2E test code. However, these changes are directly related to NFS enablement and are a reasonable accompaniment. The dynamic-plugins-app-next.yaml file is the first of its kind in the repo (other workspaces share a single dynamic-plugins.yaml or rely on framework auto-generation), but this is justified by the homepage plugin's unique routing and mount-point differences between legacy and NFS modes.


No blocking findings. The change follows established conventions for NFS enablement across the repository.

Previous run (2)

Review — approve

This PR adds NFS (New Frontend System / app-next) E2E test coverage for the homepage plugin. The implementation is well-structured, properly isolates legacy and NFS test paths, and correctly guards unsupported NFS features with test.skip.

What the PR does

  • Adds a homepage-app-next Playwright project that deploys RHDH with NFS enabled in a separate namespace, running the same spec file with NFS-aware branching via isHomepageAppNext(namespace).
  • Creates dynamic-plugins-app-next.yaml with NFS-specific plugin configuration — disabling inherited/conflicting plugins and enabling pinned GHCR artifacts with clear inline comments explaining each decision.
  • Updates dynamic-plugins.yaml (legacy) — adds Keycloak backend-module config, bumps homepage OCI from 1.17.01.17.1, and adds the upstream backstage-plugin-home OCI.
  • Extends DynamicHomePagePo page object with NFS-aware widget labels, a custom loginAsKeycloakUser that tolerates slow NFS cold loads (240s Sign In wait), clearHomeLayoutStorage for user-session isolation, and NFS edit-mode quirk handling (nudgeLayoutToEnableSave).
  • Adds NFS documentation to the catalog entity (home-page.yaml) installation section and a new appConfigExamples entry to the package metadata.

Findings

Severity Category File Description
low dead-code homepage.spec.ts Duplicate test.runOnce("homepage-keycloak-groups", ...) call

Duplicate test.runOnce callhomepage.spec.ts beforeAll calls test.runOnce("homepage-keycloak-groups", async () => { await setupKeycloakGroups(); }) twice: once before the deploy block and again after it (identical comment and key). The second call is always a no-op since runOnce skips callbacks for already-executed keys. This appears to be a copy-paste leftover from the refactoring that split the original single homepage-setup runOnce into separate Keycloak and deploy blocks. Consider removing the second occurrence.

Positive observations

  • Clean separation between NFS and legacy paths — isAppNext flag propagates correctly through the page object and test spec, with test.skip guards covering all NFS-unsupported features (server defaultWidgets, persona-based homepages).
  • Robust NFS login — the custom loginAsKeycloakUser handles slow NFS cold loads by waiting for the Sign In button with a 240s timeout, then waits for nav links after authentication. This avoids the race condition documented in the JSDoc.
  • Good use of clearHomeLayoutStorage — properly isolates user sessions in the shared browser context by clearing NFS/legacy CustomHomepageGrid localStorage keys between account switches.
  • Well-documented plugin configdynamic-plugins-app-next.yaml comments explain each disable/enable decision with references to the source workspace metadata.
  • waitForTimeout elimination — several waitForTimeout calls replaced with proper Playwright assertions (waitFor, toHaveCount, toBeVisible), improving test reliability.
  • Nightly mode guardhomepage-app-next is properly skipped in nightly mode since NFS isn't ready for nightly CI yet.
Previous run (3)

Review — ✅ Approve

PR: chore(e2e): run homepage tests with nfs
Risk: Low — E2E test infrastructure changes, no production code or plugin source modifications.

Summary

This PR adds NFS (new frontend system / app-next) E2E test support for the homepage plugin workspace. It introduces a second Playwright project (homepage-app-next), NFS-specific dynamic plugin configuration, widget label mapping between legacy Scalprum and NFS dialogs, a custom login helper with longer timeouts for slow NFS cold loads, and localStorage cleanup for cross-user layout isolation. Tests that rely on homepage-backend defaultWidgets (not yet supported on NFS) are properly skipped with test.skip. Documentation and metadata are updated with NFS configuration examples.

Reviewed dimensions

Correctness

  • The new homepage-app-next project correctly uses namespace suffix detection (-app-next) to toggle NFS behavior — consistent with how e2e-test-utils merges NFS defaults.
  • test.skip annotations correctly gate NFS-unsupported features (defaultWidgets, persona-based homepages) and nightly mode.
  • The custom loginAsKeycloakUser helper properly waits for the Sign In button with a 240s timeout before clicking, addressing the NFS cold-load race condition documented in the JSDoc.
  • clearHomeLayoutStorage() correctly targets home.customHomepage and related localStorage keys to prevent layout leakage between users in shared browser contexts.
  • setupKeycloakGroups now validates KEYCLOAK_BASE_URL with a clear error message and falls back to local defaults (admin/admin123) — a strict improvement over the previous ! non-null assertions.
  • Version bump from bs_1.52.0__1.17.0 to bs_1.52.0__1.17.1 in legacy dynamic-plugins.yaml matches the current metadata.

Security

  • No secrets exposed; Keycloak test passwords carry // gitleaks:allow comments.
  • Admin credential defaults (admin/admin123) are only used for local e2e-test-utils Keycloak instances. CI uses Vault-injected secrets.
  • No RBAC, auth, or privilege escalation concerns in test code.

Intent & coherence

  • Cleanly scoped to E2E test infrastructure for the homepage workspace — no production code, CI config, or unrelated workspace changes.
  • The approach of running both legacy and NFS projects in the same spec file with conditional test.skip is consistent with the repo's E2E patterns.
  • NFS documentation additions in the catalog entity and metadata are accurate and align with the test configuration.

Style / conventions

  • Code follows existing patterns: test.runOnce for expensive setup, WorkspacePaths.resolve for config paths, DynamicHomePagePo page-object pattern.
  • ESLint disable comments include justifications (-- NFS skips the prior login test, -- dialog open).
  • One minor style issue: duplicate test.runOnce("homepage-keycloak-groups", ...) call in beforeAll — the identical block appears before and after the deploy block. The second call is a no-op (same key, file-based flag already set). This is dead code, likely a copy-paste artifact. Harmless but should be cleaned up.

Documentation currency

  • home-page.yaml catalog entity description updated with NFS configuration instructions and app.extensions examples.
  • Homepage metadata appConfigExamples extended with NFS layout config including widgetLayout breakpoints.
  • In-code JSDoc comments explain the NFS login race condition and localStorage cleanup rationale.

Findings

# Severity File Description
1 low homepage.spec.ts Duplicate test.runOnce calltest.runOnce("homepage-keycloak-groups", ...) appears twice in beforeAll (before and after the deploy block). The second invocation is dead code since runOnce skips when the same key was already executed. Remove the duplicate to reduce confusion.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Aug 11, 2026
@github-actions github-actions Bot added mandatory-workspace PR affects a workspace with required plugins for releases workspace-update PR modifies files in an existing workspace labels Aug 11, 2026
@debsmita1

Copy link
Copy Markdown
Member Author

/publish

@github-actions

Copy link
Copy Markdown
Contributor

Publish workflow has completed with success.

Publishing process

✅ Finished successfully.

✅ Published container images:

  • ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-homepage:pr_3234__1.17.1
  • ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-homepage-backend:pr_3234__0.4.0

Backstage Compatibility Check

✅ All workspaces are compatible with the target Backstage version (1.52.0).

No action required.

Metadata Validation

✅ All metadata files validated successfully.

Running e2e tests
/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

Smoke tests workflow passed. All plugins loaded successfully.

@rhdh-test-bot

Copy link
Copy Markdown

❌ Failed E2E Tests - homepage

Platform: ocp 4.20 | RHDH Version: 1.11 | Duration: 7m 12s
Passed: 18 | Failed: 1 | Flaky: 0 | Skipped: 21
Playwright Report | Build Log | Logs | Artifacts

@debsmita1
debsmita1 force-pushed the homepage-enable-nfs-e2e branch from c4d7b0b to 818de9c Compare August 14, 2026 10:09
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 10:10 AM UTC · Ended 10:11 AM UTC

Commit: 818de9c · View workflow run →

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
@debsmita1
debsmita1 force-pushed the homepage-enable-nfs-e2e branch from 818de9c to 47eb112 Compare August 14, 2026 10:11
rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
@debsmita1
debsmita1 force-pushed the homepage-enable-nfs-e2e branch from 47eb112 to b6aa7e1 Compare August 14, 2026 10:15
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 10:16 AM UTC · Ended 10:21 AM UTC

Commit: b6aa7e1 · View workflow run →

@sanketpathak

Copy link
Copy Markdown
Contributor

/publish

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Aug 14, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:16 AM UTC · Completed 10:21 AM UTC

Commit: b6aa7e1 · View workflow run →

@github-actions

Copy link
Copy Markdown
Contributor

Publish workflow has completed with success.

Publishing process

✅ Finished successfully.

✅ Published container images:

  • ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-homepage:pr_3234__1.17.1
  • ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-homepage-backend:pr_3234__0.4.0

Backstage Compatibility Check

✅ All workspaces are compatible with the target Backstage version (1.52.0).

No action required.

Metadata Validation

✅ All metadata files validated successfully.

Running e2e tests
/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

Smoke tests workflow passed. All plugins loaded successfully.

Comment thread workspaces/homepage/e2e-tests/tests/config/dynamic-plugins.yaml Outdated
Comment thread workspaces/homepage/e2e-tests/tests/config/dynamic-plugins-app-next.yaml Outdated
Comment thread workspaces/homepage/e2e-tests/tests/config/dynamic-plugins-app-next.yaml Outdated
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:34 AM UTC · Completed 10:48 AM UTC

Commit: f12f73b · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Aug 14, 2026
@debsmita1

Copy link
Copy Markdown
Member Author

/publish

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
@debsmita1
debsmita1 force-pushed the homepage-enable-nfs-e2e branch from f12f73b to 3c88b9f Compare August 14, 2026 12:42
@debsmita1
debsmita1 requested a review from sanketpathak August 14, 2026 12:42
@github-actions

Copy link
Copy Markdown
Contributor

Publish workflow has completed with success.

Publishing process

✅ Finished successfully.

✅ Published container images:

  • ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-homepage:pr_3234__1.17.1
  • ghcr.io/redhat-developer/rhdh-plugin-export-overlays/red-hat-developer-hub-backstage-plugin-homepage-backend:pr_3234__0.4.0

Backstage Compatibility Check

✅ All workspaces are compatible with the target Backstage version (1.52.0).

No action required.

Metadata Validation

✅ All metadata files validated successfully.

Running e2e tests
/test e2e-ocp-helm

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 12:43 PM UTC · Ended 1:02 PM UTC

Commit: 3c88b9f · View workflow run →

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Smoke tests workflow passed. All plugins loaded successfully.

@rhdh-test-bot

Copy link
Copy Markdown

❌ Failed E2E Tests - homepage

Platform: ocp 4.20 | RHDH Version: 1.11 | Duration: 9m 40s
Passed: 23 | Failed: 1 | Flaky: 0 | Skipped: 16
Playwright Report | Build Log | Logs | Artifacts

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels Aug 14, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:43 PM UTC · Completed 1:02 PM UTC

Commit: 3c88b9f · View workflow run →

@sanketpathak

Copy link
Copy Markdown
Contributor

/test e2e-ocp-helm

@rhdh-test-bot

Copy link
Copy Markdown

❌ Failed E2E Tests - homepage

Platform: ocp 4.20 | RHDH Version: 1.11 | Duration: 9m 17s
Passed: 23 | Failed: 1 | Flaky: 0 | Skipped: 16
Playwright Report | Build Log | Logs | Artifacts

@openshift-ci

openshift-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

@debsmita1: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm 3c88b9f link false /test e2e-ocp-helm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Labels

mandatory-workspace PR affects a workspace with required plugins for releases requires-manual-review Review requires human judgment workspace-update PR modifies files in an existing workspace

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants