chore(e2e): scorecard grouped metric tests

[teknaS47]

Hey, I just made a Pull Request!

Adding e2e for Scorecard grouped metrics.
RHIDP-15177

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or Updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)

[rhdh-gh-app]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
app-legacy	workspaces/scorecard/packages/app-legacy	none	v0.0.0

[rhdh-qodo-merge]

PR Summary by Qodo

Add E2E coverage for Scorecard grouped metrics (NFS mode)

🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add Playwright E2E tests for Scorecard metric group cards and data sources dialog.
• Adjust SonarQube metric visibility assertions for grouped vs ungrouped layouts (APP_MODE=nfs).
• Enable grouped grid layout in scorecard app-config and extend Axe helper options.

Diagram

graph TD
  T["scorecard.test.ts"] --> P["ScorecardPage.ts"] --> UI["Scorecard UI (group cards)"]
  T --> A["accessibility.ts (axe)"]
  T --> M["Mock SonarQube API"] --> UI
  C["app-config.yaml (groups)"] --> UI
  TS["tsconfig.json"] --> T

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Feature-detect grouped layout instead of APP_MODE branching

• ➕ Less coupling to environment variables; tests adapt to whichever layout is rendered
• ➕ Avoids keeping a hardcoded groupedMetricIds list in the test
• ➖ Can mask misconfiguration (e.g., grouped layout unexpectedly enabled/disabled) unless explicitly asserted
• ➖ Requires careful detection to avoid flaky “first render” timing issues

2. Split grouped-metrics tests into a dedicated Playwright project (NFS-only)

• ➕ Cleaner tests with no per-test branching; clearer CI matrix by app mode
• ➕ Easier to keep grouped layout configuration isolated to that project
• ➖ Adds Playwright config/CI complexity and potentially longer runtime
• ➖ Requires maintaining multiple execution targets locally and in CI

Recommendation: The current approach (explicit APP_MODE=nfs gating + targeted grouped-layout assertions) is reasonable for introducing coverage quickly and keeping the grouped UI tests NFS-only. If grouped-metrics coverage expands, consider moving NFS-specific tests into a dedicated Playwright project to reduce branching and make failures easier to interpret.

Files changed (5) +309 / -44

Tests (3) +280 / -43

ScorecardPage.tsAdd page-object helpers for MetricGroupCard interactions +56/-2

Add page-object helpers for MetricGroupCard interactions

• Introduces Playwright Locator-based helpers to find group cards, bucket tiles, and interact with the data sources dialog (open/close, filter pills, table rows). Also adds message evaluation support for dynamic dialog titles.

workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts

scorecard.test.tsAdd grouped metric card E2E suite and NFS-aware SonarQube assertions +220/-41

Add grouped metric card E2E suite and NFS-aware SonarQube assertions

• Updates existing SonarQube metric visibility/value tests to account for grouped vs ungrouped layouts when APP_MODE=nfs. Adds a new NFS-only describe block validating group card rendering, bucket counts, data sources dialog behavior, filter pill filtering, and pre-applied filters when clicking bucket tiles.

workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts

accessibility.tsAllow disabling specific Axe rules in accessibility helper +4/-0

Allow disabling specific Axe rules in accessibility helper

• Extends runAccessibilityTests options with disableRules and wires it into AxeBuilder.disableRules. This supports selectively ignoring rules (e.g., color-contrast) for specific UI surfaces under test.

workspaces/scorecard/packages/app-legacy/e2e-tests/utils/accessibility.ts

Other (2) +29 / -1

app-config.yamlEnable Scorecard grid layout with metric grouping definitions +27/-0

Enable Scorecard grid layout with metric grouping definitions

• Adds a Scorecard grid-layout extension configuration defining three metric groups (Security Vulnerabilities, Code Quality, SonarQube Coverage). This enables grouped metric cards in the scorecard workspace to support the new E2E coverage.

workspaces/scorecard/app-config.yaml

tsconfig.jsonInclude E2E tests in the scorecard workspace TypeScript project +2/-1

Include E2E tests in the scorecard workspace TypeScript project

• Adds packages/*/e2e-tests to tsconfig include paths so E2E sources are typechecked/recognized by tooling.

workspaces/scorecard/tsconfig.json

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:40 AM UTC · Completed 3:57 AM UTC
Commit: 2cef413 · View workflow run →

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Broken getByLabel locators ⊘ Outdated 🐞 Bug ≡ Correctness

Description

The grouped-metrics e2e assertions use page.getByLabel('…') to locate MetricGroupCard titles, but
MetricGroupCard/CardWrapper do not set an aria-label (or associated <label>) for the card title,
so these locators will not match and the NFS-mode test path will fail.

Code

workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[R356-358]

+        await expect(page.getByLabel('Security Vulnerabilities')).toBeVisible();
+        await expect(page.getByLabel('Code Quality')).toBeVisible();
+        await expect(page.getByLabel('SonarQube Coverage')).toBeVisible();

Relevance

●●● Strong

Deterministic test-breaker: getByLabel won’t match without aria-label; team fixes
locator/accessibility issues.

PR-#3366

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

MetricGroupCard passes the group title to CardWrapper but does not set aria-label for it;
CardWrapper renders the title as visible text inside CardHeader without labeling the card. The
only aria-label in the group card is on the bucket tiles and is formatted as "{count}
{bucket.label}", so getByLabel('Security Vulnerabilities')/etc cannot match anything.

workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/MetricGroupCard.tsx[77-92]
workspaces/scorecard/plugins/scorecard/src/components/Common/CardWrapper.tsx[52-70]
workspaces/scorecard/plugins/scorecard/src/components/MetricGroupCard/ThresholdBucketTile.tsx[49-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Playwright `getByLabel('Security Vulnerabilities' | 'Code Quality' | 'SonarQube Coverage')` is used to assert group card visibility. Those strings are rendered as visible text (CardHeader title), but there is no `aria-label`/label association for them, so `getByLabel` will not find the cards.

### Issue Context
`MetricGroupCard` renders a `CardWrapper` with `role="article"` and a `CardHeader` showing the title as text, but no `aria-label` is applied. The only `aria-label` in the card subtree is on the threshold bucket tiles (`"{count} {bucket.label}"`), not the group title.

### Fix Focus Areas
- workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[356-358]
- workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[400-402]
- workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[1139-1142]
- workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[1149-1173]

### Implementation notes
- Replace `page.getByLabel('<Group Title>')` assertions with a locator that matches the actual DOM, e.g.:
 - `await expect(scorecardPage.getGroupCard('<Group Title>')).toBeVisible();` (preferred, since helper already exists)
 - or `page.locator('[role="article"]').filter({ hasText: '<Group Title>' }).first()`
- Keep `getByLabel(...)` for controls that truly use labels/`aria-label` (e.g., menu button uses `ariaLabel={t('metricGroupCard.menuAriaLabel')}`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Hardcoded group card strings ⊘ Outdated 📘 Rule violation ⚙ Maintainability

Description

New e2e tests assert/select using hardcoded UI text like Security Vulnerabilities, Code Quality,
and Track security issues across your repositories instead of using translation keys/messages,
making tests locale-fragile. This violates the requirement to use translation-loaded strings in
selectors/assertions.

Code

workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[R1139-1141]

+      await page.getByText('Scorecard', { exact: true }).click();
+      await expect(page.getByLabel('Security Vulnerabilities')).toBeVisible({
+        timeout: 15000,

Relevance

●●● Strong

Team commonly replaces hardcoded UI strings with translations/i18n interpolation to avoid locale
fragility.

PR-#3446
PR-#3417

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2596 requires using translation keys/messages rather than hardcoded UI strings in
e2e selectors/assertions. The added tests include multiple literal strings for group card titles and
descriptions (and navigation text) in getByText, getByLabel, and getGroupCard(...) calls.

Rule 2596: E2E tests must use translation keys instead of hardcoded UI strings
workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[1139-1165]
workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[356-358]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly-added Scorecard grouped-metrics e2e tests use hardcoded user-facing strings (titles/descriptions) in selectors and assertions.

## Issue Context
Compliance requires e2e tests to use translation keys/messages (via `getTranslations(...)`) rather than literal English UI text, so tests remain stable across locales.

## Fix Focus Areas
- workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[1139-1176]
- workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts[356-358]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. evaluateMessage() used for templates 📘 Rule violation ⚙ Maintainability

Description

New code uses evaluateMessage(...) for {{...}} placeholder substitution instead of the required
replaceTemplate(template, values) helper. This diverges from the mandated template-substitution
approach for e2e tests.

Code

workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts[R148-152]

+  getDialogTitle(groupTitle: string): string {
+    return evaluateMessage(
+      this.translations.dataSourcesDialog.title,
+      groupTitle,
+    );

Relevance

●● Moderate

Scorecard e2e already accepted evaluateMessage-based templating; unclear if replaceTemplate is
enforced here.

PR-#3245

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2646 requires using a replaceTemplate helper for translation templates with
{{...}} placeholders. The new getDialogTitle() uses evaluateMessage(...) to substitute into
translations.dataSourcesDialog.title, which is a placeholder template string.

Rule 2646: Use replaceTemplate helper for template string placeholder substitution in e2e tests
workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts[148-152]
workspaces/scorecard/plugins/scorecard/src/translations/ref.ts[181-184]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Template placeholder substitution in e2e code is implemented via `evaluateMessage(...)` instead of the required `replaceTemplate(template, values)` helper.

## Issue Context
The scorecard translation `dataSourcesDialog.title` is a template string (`'{{title}} sources'`). Compliance requires using a shared `replaceTemplate(...)` helper for placeholder substitution in e2e tests.

## Fix Focus Areas
- workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts[148-152]
- workspaces/scorecard/packages/app-legacy/e2e-tests/utils/translationUtils.ts[102-114]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 11 rules

✅ Cross-repo context

  Not relevant to this PR: redhat-developer/rhdh

  Not relevant to this PR: redhat-developer/rhdh-chart

  Not relevant to this PR: redhat-developer/rhdh-operator

  Not relevant to this PR: redhat-developer/rhdh-local

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Review

Findings

Medium

• [page-object-method-consistency] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard-nfs.test.ts:221 — The test "Verify quality gate failure shows error bucket on Code Quality group card" uses raw locators (page.locator('[role="article"]').filter(...)) directly in the test body instead of using the ScorecardPage.getGroupCard() and ScorecardPage.getBucketTile() page object methods introduced in this same PR. All other tests in this file use the page object helpers.
  Remediation: Replace inline locators with scorecardPage.getGroupCard('Code Quality') and scorecardPage.getBucketTile(card, 'error').

Low

• [test-data-consistency] workspaces/scorecard/app-config.yaml:50 — The metric sonarqube.securityRating appears in both the security-vulnerabilities group and the sonarqube-coverage group. This may be intentional test data designed to verify a metric can belong to multiple groups, but could confuse users who reference this config as an example.

• [test-adequacy] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard-nfs.test.ts:148 — The "Verify clicking bucket tile opens dialog with filter pre-applied" test uses a generic [aria-pressed="true"] selector to check that a pill is active but does not verify which specific pill is pre-selected. The test does verify that all table rows contain the warning threshold text, providing indirect validation.

• [scope-creep] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — Beyond adding the test.skip guard for NFS mode, this file also renames variables, simplifies map calls, reorders entries, and changes the quality gate card locator strategy. These are reasonable cleanup but extend the diff footprint beyond the stated scope.

• [naming-convention] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard-nfs.test.ts:29 — The top-level test.describe block is named "Metric Group Cards" while the existing file uses "Scorecard Plugin Tests", lacking symmetry with the existing naming convention.

• [comment-style] workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts:113 — Section comment // Metric Group Card helpers is a new pattern not used elsewhere in the class.

• [variable-declaration-scope] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard-nfs.test.ts:40 — currentLocale is declared as a local variable inside beforeAll rather than at describe-block scope like the existing scorecard.test.ts. This is acceptable since it is not used outside beforeAll in this file.

Previous run

Review

Findings

Low

• [test-inadequate] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts:372 — The legacyOnlyValues naming in the 'Verify SonarQube metric values' test suggests a mode-dependent split, but the entire SonarQube Entity Scorecards describe block is skipped in NFS mode via test.skip. Both expectedValues and legacyOnlyValues always run together in legacy mode only, making the naming distinction misleading. The naming may serve as forward-looking documentation of intent.

• [scope-creep] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts:350 — The existing scorecard.test.ts has modifications beyond the NFS skip guard: assertion reorganization and locator strategy changes in the SonarQube tests. These are reasonable adaptations for NFS mode coexistence but extend beyond the stated chore scope.

• [scope-creep] workspaces/scorecard/packages/app-legacy/e2e-tests/utils/accessibility.ts:26 — The disableRules option added to the accessibility utility is a generic enhancement beyond the stated scope, used once to disable color-contrast in the data-sources dialog test.

• [authorization-traceability] — The PR references Jira RHIDP-15177 but has no linked GitHub issue. The change is purely additive e2e tests plus the app-config fixture, consistent with a test-only chore.

Previous run (2)

Review

Findings

Medium

• [return-type-consistency] workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts:132 — The method openDataSourcesDialog returns Promise<Locator>, deviating from the established page object convention where async methods return Promise<void> and Locator-returning methods are synchronous. In HomePage, ScorecardDrillDownPage, and CatalogPage, no existing async method returns a Locator.
  Remediation: Consider splitting into two methods: async openDataSourcesDialog(card: Locator): Promise<void> for the click actions, and getDialog(): Locator returning this.page.locator('[role="dialog"]'). The visibility assertion can stay in the action method.

Previous run (3)

Review

Findings

Low

• [edge-case] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts:1267 — The "Verify clicking bucket tile opens dialog with filter pre-applied" test checks that some pill has aria-pressed="true" but does not verify which specific filter pill is active. The locator dialog.locator('[role="button"][aria-pressed="true"]') could match any pressed pill. If the implementation incorrectly pre-applies a different filter (e.g., "Success" instead of "Warning"), the test would still pass. Consider asserting the active pill contains the expected threshold text (e.g., translations.thresholds.warning).

Previous run (4)

Review

Findings

Low

• [test-isolation] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — The 'Metric Group Cards' describe block uses test.beforeAll to navigate to a specific page and relies on sequential test execution with shared page state. If a test fails after opening a dialog but before closing it, subsequent tests will fail because they assume a clean starting state. This is consistent with existing test patterns in this file.

Previous run (5)

Review

Findings

Low

• [test coverage gap] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — In the 'Verify SonarQube metric values' test, the NFS branch only asserts that the 'Code Quality' group label is visible but does not verify per-metric values (qualityGate, codeCoverage, maintainabilityRating, openIssues, securityHotspots, securityRating) within the group cards. The separate 'Metric Group Cards' test suite verifies aggregate bucket tile counts but not individual metric values. See also: [scope-coherence] finding.

• [scope-coherence] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — The PR adapts three existing SonarQube tests with isNfs branching, which reduces what is verified when APP_MODE=nfs: six metric value checks move to legacy-only branches, and the quality gate failure test checks for an error bucket instead of the DangerousOutlinedIcon. Consider noting these adaptations in the PR description so reviewers can explicitly evaluate the NFS-mode assertion changes. See also: [test coverage gap] finding.

• [type alias reuse] workspaces/scorecard/packages/app-legacy/e2e-tests/pages/ScorecardPage.ts:124 — The threshold key union 'success' | 'warning' | 'error' is inlined in getBucketTile and getFilterPill. In HomePage.ts, the same union is extracted to a named ThresholdState type alias. Consider extracting a shared type to maintain consistency with sibling page objects.

Previous run (6)

Review

Findings

Medium

• [test-coverage-regression] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — The 'Verify SonarQube metric values' test unconditionally removes 5 metrics (openIssues, securityRating, securityHotspots, maintainabilityRating, codeCoverage) from the expectedValues map. These are the metrics assigned to NFS metric groups, but in non-NFS (legacy) mode they still render as individual cards and their values should still be checked. The removals should be conditional on isNfs.
  Remediation: Build the expectedValues map conditionally — include all 11 metrics for non-NFS mode, and only the 6 ungrouped metrics for NFS mode.

• [test-assertion-weakening] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — In the 'Verify SonarQube quality gate failure state' test, the non-NFS else branch now only asserts that qualityGateCard is visible, whereas the base code also asserted that DangerousOutlinedIcon was visible to verify failure-state icon rendering.
  Remediation: Restore the DangerousOutlinedIcon assertion in the non-NFS else branch.

Low

• [test-assertion-weakening] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — In the 'Verify SonarQube metric values' test, the non-NFS else branch for the qualityGate card check dropped the CheckCircleOutlineIcon assertion that verified success-state icon rendering.

• [inconsistent-conditional-check] workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts — The 'Metric Group Cards' describe block uses process.env.APP_MODE !== 'nfs' directly in test.skip(), while the sibling 'SonarQube Entity Scorecards' block defines const isNfs. Hoisting isNfs to the parent test.describe('Scorecard Plugin Tests') scope would avoid duplication.

---

Labels: PR adds e2e tests for scorecard grouped metrics

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:04 AM UTC · Completed 8:17 AM UTC

Commit: 29dc346 · View workflow run →

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 10:06 AM UTC · Ended 10:20 AM UTC

Commit: 0a7a98c · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:06 AM UTC · Completed 10:20 AM UTC

Commit: 0a7a98c · View workflow run →

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 12:12 PM UTC · Ended 12:27 PM UTC

Commit: 29704e1 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:12 PM UTC · Completed 12:27 PM UTC

Commit: 29704e1 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 2:02 PM UTC · Ended 2:02 PM UTC

Commit: 57589d6 · View workflow run →

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:03 PM UTC · Completed 2:19 PM UTC

Commit: ad5d150 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.