Skip to content

RHDHBUGS-3057: Refactor e2e tests for the scorecard plugin#3245

Open
imykhno wants to merge 3 commits into
redhat-developer:mainfrom
imykhno:scorecard/implement-e2e-tests
Open

RHDHBUGS-3057: Refactor e2e tests for the scorecard plugin#3245
imykhno wants to merge 3 commits into
redhat-developer:mainfrom
imykhno:scorecard/implement-e2e-tests

Conversation

@imykhno
Copy link
Copy Markdown
Contributor

@imykhno imykhno commented May 27, 2026

Hey, I just made a Pull Request!

This refactoring follows the merge of #2923. The expectation was that tests will be reviewed and added those tests that are missing to test scorecard aggregation card customization. Additionally, this PR incorporates the feedback and comments raised during the review of #2923.

PR is for:

✔️ 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)

…functions

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented May 27, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Aggregation wait logic inverted ✓ Resolved 🐞 Bug ≡ Correctness
Description
waitForAggregationResponse treats mismatches as “valid” (uses !==) and then rejects matching
responses, so any use with options.expectedResult will hang until timeout. This makes the helper
unusable for value-based waiting and will cause future tests to intermittently stall or always fail
when they start using expectedResult.
Code

workspaces/scorecard/packages/app-legacy/e2e-tests/utils/apiUtils.ts[R86-94]

Relevance

⭐⭐⭐ High

Team fixes e2e race/flakiness; similar stabilization changes merged (race fix #2558; e2e hardening
#2703).

PR-#2558
PR-#2703

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The predicate marks a response as valid only when the returned values differ from the expected ones,
and then returns false when either check is not "valid", which rejects the matching response and
causes an eventual timeout.

workspaces/scorecard/packages/app-legacy/e2e-tests/utils/apiUtils.ts[50-103]

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

### Issue description
`waitForAggregationResponse` currently rejects responses that match the provided `expectedResult` because it checks `result != expected` and requires those checks to be "valid". This makes the predicate unsatisfiable when `expectedResult` is provided.

### Issue Context
This helper is exported and intended for synchronizing tests on specific aggregation payloads. With the current logic, any future test that passes `expectedResult` will time out.

### Fix Focus Areas
- workspaces/scorecard/packages/app-legacy/e2e-tests/utils/apiUtils.ts[50-103]

### Suggested change
Rewrite the checks as:
- `averageOk = expected.averageScore === undefined || result?.averageScore === expected.averageScore`
- `totalOk = expected.total === undefined || result?.total === expected.total`
- `return averageOk && totalOk`

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



Remediation recommended

2. Homepage waits can resolve early ✓ Resolved 🐞 Bug ☼ Reliability
Description
setupHomepageAllCardsNoData creates waitForAggregationResponse promises before adding/saving
widgets, so those waits can be satisfied by pre-reload requests and not guarantee that the
post-page.reload() fetches finished. This undermines the purpose of waiting for homepage cards to
finish loading and can introduce test flakiness during reload-dependent assertions.
Code

workspaces/scorecard/packages/app-legacy/e2e-tests/utils/homepageWidgetUtils.ts[R74-83]

Relevance

⭐⭐⭐ High

E2E reliability issues around timing/reload are historically addressed and merged (#2144, #2558,
#2703).

PR-#2144
PR-#2558
PR-#2703

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code constructs the wait promises before the actions that can already trigger aggregation
fetches (navigate/edit/save). Because the waits are not tied to the reload call, they can resolve
before the reload-triggered requests complete.

workspaces/scorecard/packages/app-legacy/e2e-tests/utils/homepageWidgetUtils.ts[70-83]
workspaces/scorecard/packages/app-legacy/e2e-tests/utils/apiUtils.ts[57-103]

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

### Issue description
`setupHomepageAllCardsNoData` sets up response waits too early, so `Promise.all(responseWaits)` may complete without actually waiting for the requests triggered by the subsequent `page.reload()`.

### Issue Context
The helper intends to stabilize the test by waiting for aggregation responses after a reload (to clear React Query cache). If the waits resolve due to earlier requests (during navigation/edit/save), the reload-triggered network activity may still be in flight when assertions run.

### Fix Focus Areas
- workspaces/scorecard/packages/app-legacy/e2e-tests/utils/homepageWidgetUtils.ts[70-83]

### Suggested change
Create waits immediately before calling `page.reload()` (after widgets are added/saved), e.g.:
1) `await addAggregatedScorecardWidgets(homePage)`
2) create `responseWaits`
3) `await Promise.all([page.reload(), ...responseWaits])`

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


Grey Divider

Qodo Logo

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app Bot commented May 27, 2026

Changed Packages

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.23%. Comparing base (dda0303) to head (1d79f52).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3245   +/-   ##
=======================================
  Coverage   53.23%   53.23%           
=======================================
  Files        2413     2413           
  Lines       86358    86358           
  Branches    23912    23913    +1     
=======================================
  Hits        45974    45974           
  Misses      38907    38907           
  Partials     1477     1477           
Flag Coverage Δ *Carryforward flag
adoption-insights 83.58% <ø> (ø) Carriedforward from dda0303
ai-integrations 70.03% <ø> (ø) Carriedforward from dda0303
app-defaults 69.60% <ø> (ø) Carriedforward from dda0303
augment 46.39% <ø> (ø) Carriedforward from dda0303
bulk-import 72.86% <ø> (ø) Carriedforward from dda0303
cost-management 16.49% <ø> (ø) Carriedforward from dda0303
dcm 32.85% <ø> (ø) Carriedforward from dda0303
extensions 61.79% <ø> (ø) Carriedforward from dda0303
global-floating-action-button 74.30% <ø> (ø) Carriedforward from dda0303
global-header 61.68% <ø> (ø) Carriedforward from dda0303
homepage 51.52% <ø> (ø) Carriedforward from dda0303
konflux 91.01% <ø> (ø) Carriedforward from dda0303
lightspeed 68.33% <ø> (ø) Carriedforward from dda0303
mcp-integrations 81.59% <ø> (ø) Carriedforward from dda0303
orchestrator 36.36% <ø> (ø) Carriedforward from dda0303
quickstart 62.88% <ø> (ø) Carriedforward from dda0303
sandbox 79.49% <ø> (ø) Carriedforward from dda0303
scorecard 83.84% <ø> (ø)
theme 64.54% <ø> (ø) Carriedforward from dda0303
translations 8.49% <ø> (ø) Carriedforward from dda0303
x2a 78.47% <ø> (ø) Carriedforward from dda0303

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dda0303...1d79f52. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…nts and improve widget handling

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Refactor scorecard e2e tests with aggregation utilities and improved test structure

🧪 Tests ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Refactored e2e test structure with new aggregation constants and utility functions
• Reorganized constants from homepageWidgetTitles.ts to aggregations.ts with improved naming
• Extracted common test setup logic into reusable homepageWidgetUtils.ts helper functions
• Enhanced API utilities with aggregation response waiting and validation capabilities
• Improved test organization by grouping related aggregation tests with shared setup
Diagram
flowchart LR
  A["Old Constants<br/>homepageWidgetTitles.ts"] -->|Migrate & Rename| B["New Constants<br/>aggregations.ts"]
  C["Scattered Test Setup<br/>Code"] -->|Extract| D["Utility Functions<br/>homepageWidgetUtils.ts"]
  E["Basic API Utils<br/>apiUtils.ts"] -->|Enhance| F["Aggregation Response<br/>Waiting & Validation"]
  B --> G["Cleaner Test Code<br/>scorecard.test.ts"]
  D --> G
  F --> G

Loading

Grey Divider

File Changes

1. workspaces/scorecard/packages/app-legacy/e2e-tests/constants/aggregations.ts ⚙️ Configuration changes +60/-0

New aggregation constants file with metadata

• New file consolidating aggregation-related constants previously scattered across files
• Defines AGGREGATED_CARDS_METRIC_IDS with metric and KPI identifiers
• Defines AGGREGATED_CARDS_WIDGET_TITLES mapping widget titles for UI interactions
• Defines AGGREGATED_CARDS_METADATA with complete aggregation configuration including id, title,
 and metricId

workspaces/scorecard/packages/app-legacy/e2e-tests/constants/aggregations.ts


2. workspaces/scorecard/packages/app-legacy/e2e-tests/constants/homepageWidgetTitles.ts Refactoring +0/-32

Removed in favor of aggregations.ts

• File deleted and replaced by aggregations.ts
• Constants migrated with improved naming conventions
• Old naming (e.g., withDeprecatedMetricId) replaced with clearer names (e.g., jiraMetricId)

workspaces/scorecard/packages/app-legacy/e2e-tests/constants/homepageWidgetTitles.ts


3. workspaces/scorecard/packages/app-legacy/e2e-tests/pages/HomePage.ts Refactoring +2/-4

Updated imports and constant references

• Updated import to use new aggregations.ts constants file
• Updated widget title reference from withOpenPrsWeightedKpi to openPrsWeightedKpi
• Simplified constant naming in card pattern matching logic

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


View more (4)
4. workspaces/scorecard/packages/app-legacy/e2e-tests/scorecard.test.ts 🧪 Tests +219/-489

Major test refactoring with utility extraction

• Extracted common test setup functions (addWidgets, addAggregatedScorecardWidgets) to
 homepageWidgetUtils.ts
• Removed test.afterEach hook that was unrouting API responses
• Reorganized test structure using test.beforeAll and test.afterAll for aggregation card setup
• Refactored test groups to use shared setupHomepageAggregationCard utility function
• Updated all constant references from old naming to new AGGREGATED_CARDS_METADATA structure
• Consolidated duplicate test logic and improved test readability with metadata-driven approach
• Added new test for empty aggregated response across all default homepage cards

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


5. workspaces/scorecard/packages/app-legacy/e2e-tests/utils/apiUtils.ts ✨ Enhancement +65/-0

Enhanced API utilities for aggregation responses

• Added Response type import from @playwright/test
• Created WaitForAggregationResponseOptions type for aggregation response validation
• Added isAggregationDataUrl helper function to identify aggregation data endpoints
• Implemented waitForAggregationResponse function to wait for specific aggregation responses with
 optional result validation

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


6. workspaces/scorecard/packages/app-legacy/e2e-tests/utils/homepageWidgetUtils.ts ✨ Enhancement +83/-0

New homepage widget utility functions

• New utility file extracting common test setup functions
• Provides addWidgets function for adding single widget to homepage
• Provides addAggregatedScorecardWidgets function for adding all aggregation widgets
• Provides setupHomepageAggregationCard function for complete card setup with mocking and reload
• Provides setupHomepageAllCardsNoData function for testing empty data scenarios across all cards

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


7. workspaces/scorecard/packages/app-legacy/e2e-tests/utils/mockHomepageAggregations.ts ✨ Enhancement +47/-0

Added mock function for empty aggregation responses

• Added imports for empty response constants (emptyGithubAggregatedResponse,
 emptyJiraAggregatedResponse, emptyOpenPrsWeightedAggregatedResponse)
• Implemented mockAggregationNoDataFound function to mock all aggregation endpoints with empty
 responses
• Function intelligently routes requests based on aggregation ID to appropriate empty response

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


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added enhancement New feature or request Tests labels May 27, 2026
Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@imykhno imykhno force-pushed the scorecard/implement-e2e-tests branch from ca1706a to 1d79f52 Compare May 27, 2026 09:13
@sonarqubecloud
Copy link
Copy Markdown

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.

1 participant