chore(scorecard): version bump to v1.51.2#3284
Conversation
Eswaraiahsapram
requested review from
a team,
christoph-jerolimov and
its-mitesh-kumar
as code owners
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
ReviewFindingsMedium
Low
Previous runReviewFindingsHigh
Medium
Previous run (2)ReviewReason: stale-head The review agent reviewed commit Previous run (3)Looks good to me ReviewFindingsLow
Previous run (4)Looks good to me ReviewFindingsLow
Previous run (5)ReviewFindingsMedium
Low
Previous run (6)ReviewFindingsMedium
Low
Previous run (7)ReviewFindingsMedium
Low
Previous run (8)ReviewFindingsLow
Previous run (9)ReviewReason: stale-head The review agent reviewed commit Previous run (10)ReviewFindingsMedium
Low
Labels: PR is a dependency version bump for the scorecard workspace Previous run (11)ReviewFindingsMedium
Low
Info
Previous run (12)ReviewFindingsCritical
Low
Previous run (13)ReviewFindingsCritical
Medium
Low
Info
Previous run (14)ReviewFindingsMedium
Low
Info
Previous run (15)ReviewFindingsLow
Info
Previous run (16)ReviewFindingsMedium
Low
Info
Previous run (17)ReviewReason: stale-head The review agent reviewed commit Previous run (18)ReviewFindingsLow
Info
Previous run (19)ReviewFindingsLow
Info
Previous run (20)ReviewFindingsMedium
Low
Info
Previous run (21)ReviewFindingsHigh
Low
Info
Previous run (22)ReviewReason: stale-head The review agent reviewed commit Previous run (23)ReviewReason: stale-head The review agent reviewed commit |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| '@red-hat-developer-hub/backstage-plugin-scorecard': minor | ||
| --- | ||
|
|
||
| Backsatge version bump to v1.51.0 |
There was a problem hiding this comment.
[low] naming-convention
Typo in changeset description: 'Backsatge version bump to v1.51.0' should be 'Backstage version bump to v1.51.0'. This text will appear in the published changelog.
Suggested fix: Fix the typo: Backsatge → Backstage.
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
4b5264d to
1cd6c9b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3284 +/- ##
==========================================
- Coverage 54.21% 54.21% -0.01%
==========================================
Files 2312 2312
Lines 88532 88534 +2
Branches 24621 24631 +10
==========================================
Hits 48001 48001
- Misses 40229 40231 +2
Partials 302 302
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
fullsend-ai-review
Bot
added
the
requires-manual-review
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
1cd6c9b to
6b6556b
Compare
|
|
||
| const displayName = getLocaleDisplayName(locale); | ||
| await this.page.getByRole('link', { name: 'Settings' }).click(); | ||
| await this.page.goto('/settings'); |
There was a problem hiding this comment.
[low] test-inadequate
E2E test replaced UI-element-based navigation with direct page.goto() calls, slightly reducing coverage of navigation link presence/functionality. The affected code is test setup, not the behavior under test.
fullsend-ai-review
Bot
added
ready-for-merge
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
6b6556b to
aad1405
Compare
|
|
||
| const displayName = getLocaleDisplayName(locale); | ||
| await this.page.getByRole('link', { name: 'Settings' }).click(); | ||
| await this.page.goto('/settings'); |
There was a problem hiding this comment.
[low] test-weakened
switchToLocale switches from role-based link clicking to direct URL navigation (page.goto), bypassing UI element verification. However, this is consistent with existing page.goto() usage in loginAndSetLocale() and openCatalog() in the same file, and the method's core purpose (locale switching) remains fully tested.
fullsend-ai-review
Bot
added
ready-for-merge
|
I think we can get in these PRs before this bump, to avoid rebasing:
|
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
aad1405 to
6548a29
Compare
|
🤖 Finished Review · ✅ Success · Started 6:27 AM UTC · Completed 6:36 AM UTC |
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
ce408c2 to
a801f33
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 6:38 AM UTC · Completed 6:46 AM UTC |
fullsend-ai-review
Bot
added
requires-manual-review
Eswaraiahsapram
changed the title
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
1ae277c to
2946aae
Compare
|
🤖 Review · ❌ Terminated · Started 12:08 PM UTC · Ended 12:22 PM UTC |
|
🤖 Finished Review · ❌ Failure · Started 12:08 PM UTC · Completed 12:22 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 12:25 PM UTC · Completed 1:01 PM UTC |
fullsend-ai-review
Bot
added
requires-manual-review
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
2946aae to
33910a0
Compare
|
🤖 Finished Review · ❌ Failure · Started 4:49 AM UTC · Completed 5:00 AM UTC |
|
🤖 Finished Review · ✅ Success · Started 5:15 AM UTC · Completed 5:25 AM UTC |
| ) : null} | ||
|
|
||
| <Tooltip content={isTooltipEnabled ? tooltipContent : () => null} /> | ||
| {isTooltipEnabled && <Tooltip content={tooltipContent as never} />} |
There was a problem hiding this comment.
[low] type-cast idiom
The as never cast is a broad type-suppression pattern not used anywhere else in this codebase, introduced to work around a Recharts Tooltip type incompatibility. The behavioral change is safe, but the cast defeats compile-time type checking.
Suggested fix: Prefer a more precise cast or adjust the tooltipContent prop type to align with Recharts' expected Tooltip.content type.
| type: 'local' # Alternatives - 'googleGcs' or 'awsS3'. Read documentation for using alternatives. | ||
|
|
||
| auth: | ||
| environment: development |
There was a problem hiding this comment.
[low] auth configuration
The guest auth provider config changes from explicit userEntityRef to guest: {} with environment: development. The RBAC admin config still references user:development/guest. Verify the default guest provider identity in Backstage v1.51.2 matches.
Suggested fix: Confirm that the default guest provider identity resolves to user:development/guest so RBAC admin permissions are preserved.
fullsend-ai-review
Bot
added
ready-for-merge
|
🤖 Finished Review · ✅ Success · Started 11:51 AM UTC · Completed 12:22 PM UTC |
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
09711b9 to
449fe63
Compare
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 12:25 PM UTC · Completed 12:34 PM UTC |
| use: { | ||
| channel: 'chrome' as const, | ||
| locale, | ||
| baseURL: `http://localhost:${FRONTEND_PORT_BASE + i}`, |
There was a problem hiding this comment.
[high] logic-error
The per-project baseURL (http://localhost:${FRONTEND_PORT_BASE + i}) is unconditionally set, which overrides the global baseURL that respects process.env.PLAYWRIGHT_URL. In Playwright, project-level use fields override global use fields, so the global baseURL: process.env.PLAYWRIGHT_URL on line 63 becomes dead code for all projects. When PLAYWRIGHT_URL is provided (e.g., CI environments pointing to an external server), each project will still attempt to connect to localhost:300x instead of the provided URL, while no webServer is started (the webServer array is empty when PLAYWRIGHT_URL is set). This is a regression from the previous config where no per-project baseURL was set, allowing the global PLAYWRIGHT_URL to take effect.
Suggested fix: Make the per-project baseURL conditional: baseURL: process.env.PLAYWRIGHT_URL ?? http://localhost:${FRONTEND_PORT_BASE + i}
| }, | ||
| "type": "object", | ||
| }, | ||
| "schema": [Function], |
There was a problem hiding this comment.
[medium] test-weakened
The inline snapshot assertion for configSchema.schema is weakened from a full JSON Schema object (32 lines of structural validation including property types, required fields, and additionalProperties constraints) to schema: [Function]. This removes validation that the config schema defines the expected groups property with correct types and required fields. While this likely reflects an upstream API change in Backstage (lazy schema getter), the loss of structural coverage means regressions in the config schema definition would go undetected by this test.
Suggested fix: Add a separate assertion that calls the schema function and validates the returned schema object, e.g., expect(result.configSchema.schema()).toMatchObject({ properties: { groups: { type: object } } })
fullsend-ai-review
Bot
removed
the
ready-for-merge
- Add proper cleanup of dialog event listener to prevent memory leaks - Use try-finally pattern to ensure cleanup happens - Fixes Node 24 E2E test failures while maintaining Node 22 compatibility
Eswaraiahsapram
force-pushed
the
scorecard-bump-1.51.0
branch
from
11f73ac to
2162b7c
Compare
|
|
🤖 Finished Review · ❌ Failure · Started 2:53 AM UTC · Completed 3:07 AM UTC |
2 participants
Hey, I just made a Pull Request!
Scorecard version bump
Fix - https://redhat.atlassian.net/browse/RHIDP-13795
Screen.Recording.2026-06-03.at.8.55.19.PM.mov
E2E Test Fixes (Playwright)
Problem: All e2e tests were failing at the guest login step after the Backstage 1.51.2 version bump. The error was:
Root Cause: In
@backstage/core-components0.18.11, the guest sign-in flow now requires the backend's/api/auth/guest/refreshendpoint to be ready before sign-in can succeed. The previousplaywright.config.tsonly waited for the frontend (port 3000) to be available, but the backend (port 7007) takes longer to start. Tests would begin before the backend was ready, causing guest auth to fail.Changes (aligned with
workspaces/adoption-insightse2e setup):playwright.config.ts— Restructured to follow the same pattern asadoption-insights:/.backstage/health/v1/readiness) instead of just a frontend port, ensuring the auth backend is fully initialized before tests runbaseURLfor locale isolationreuseExistingServer: falsefor CI reliabilitye2e-tests/test_yamls/app-config-e2e-{locale}.yaml(6 files) — Same structure asadoption-insights/e2e-tests/test_yamls/. Per-locale config overrides that assign unique frontend/backend ports so each locale runs in isolation:How to test
app-config.yamlapp-config.yamlapp-config.yaml✔️ Checklist