Skip to content

test(backend): add Layer 1+2 tests for backend plugins and auth resolvers (RHIDP-13233)#4863

Open
gustavolira wants to merge 8 commits into
redhat-developer:mainfrom
gustavolira:RHIDP-13233-backend-layer1-2-tests
Open

test(backend): add Layer 1+2 tests for backend plugins and auth resolvers (RHIDP-13233)#4863
gustavolira wants to merge 8 commits into
redhat-developer:mainfrom
gustavolira:RHIDP-13233-backend-layer1-2-tests

Conversation

@gustavolira
Copy link
Copy Markdown
Member

@gustavolira gustavolira commented May 20, 2026

Description

Implements RHIDP-13233 — Layer 1 + Layer 2 backend tests, the first story of the RHIDP-11861 / RHIDP-13496 Test Layer Infrastructure work.

Establishes fast (<10 min), cluster-free unit/integration tests for RHDH-owned backend code so PR feedback can gate merges without provisioning a cluster.

4-Layer model (scope of this story)

  • Layer 1 — Unit/router tests with mockServices + supertest (no DB, no cluster)
  • Layer 2 — Integration tests with startTestBackend + in-memory wiring (real plugin boot, no cluster)
  • Layer 3 (component) and Layer 4 (E2E) are out of scope (tracked separately).

Changes (test-only — no production code touched)

Backend plugins — all 3 RHDH-owned plugins, now 100% covered

licensed-users-info-backend (5 suites) — router.test.ts: /health, /users/quantity, /users (JSON, CSV, no-catalog-match, conversion-error, 403), and the SQLite in-memory auto-disable branch. New readBackstageTokenExpiration.test.ts (default / clamp / boundaries), catalogStore.test.ts (ref mapping + filtering + on-behalf-of token), databaseUserInfoStore.test.ts (count + error). New router.integration.test.ts (Layer 2).

dynamic-plugins-info-backend (2 suites) — router.test.ts refactored to a stub against the public plugins() contract; installer-stripping, empty-list, front-end-plugin and auth-enforcement tests. New router.integration.test.ts (Layer 2 via mocked dynamicPluginsServiceRef).

scalprum-backend (2 suites) — "no matching package" warn-branch unit test and new router.integration.test.ts (Layer 2, unauthenticated policy).

Backend app auth sign-in resolvers (packages/backend, previously 0% coverage)

resolverUtils.test.tscreateOidcSubClaimResolver (sub / id-token / mismatch / no-sub-in-token / fallback) and trySignInResolvers (first-success, skip-then-succeed, all-fail, with per-resolver call-count assertions).

rhdhSignInResolvers.test.ts — preferred-username, oauth2-proxy header precedence (OAUTH_USER_HEADERx-forwarded-preferred-usernamex-forwarded-user), LDAP uuid matching (custom key, missing/mismatch), Keycloak and Ping Identity sub-claim wiring, dangerous fallback options.

Coverage

Every touched backend source file reports 100% statements/branches/functions/lines:

Area Files at 100%
licensed-users-info-backend router.ts, plugin.ts, catalogStore.ts, databaseUserInfoStore.ts, readBackstageTokenExpiration.ts
dynamic-plugins-info-backend router.ts, plugin.ts
scalprum-backend router.ts, plugin.ts
packages/backend resolverUtils.ts, rhdhSignInResolvers.ts

Notes

  • SQLite in-memory caveat: licensed-users-info auto-disables under pure in-memory SQLite. Providing backend.database.connection.directory: ':memory:' keeps it enabled while staying cluster-free (documented inline in the integration test).
  • All tests run via the existing backstage-cli package test / turbo setup — no new tooling or CI changes required.

Testing

  • yarn tsc passes for all affected packages
  • Lint / prettier pass
  • Test with Node.js (required check) green — turbo --affected runs the touched packages
  • 100% coverage on every touched backend source file

Codecov configuration

Also adds a component_management section to codecov.yml (the comment layout
already referenced components but none were defined). This gives a per-area
coverage breakdown — backend plugins, backend app, frontend app, plugin-utils,
dynamic-plugins utils — in the dashboard and PR comment. Components are
path-based views over existing uploads (no CI changes); statuses are
informational while coverage matures. Validated via codecov.io/validate.

…n pilot

Implements RHIDP-13233 (Test Layer Infrastructure epic, RHIDP-13496).

Layer 1 (unit/router):
- licensed-users-info: cover /users/quantity (200 + 403), /users (200, CSV,
  403), and add direct unit tests for rowToResponse (SQL/ISO/invalid dates)
  and permissionCheck (ALLOW/DENY). router.ts coverage 0 -> 94.6%.
- licensed-users-info: new readBackstageTokenExpiration unit tests (default,
  in-range, min/max clamp). 100% covered.
- dynamic-plugins-info: assert installer stripping, empty list, and auth
  enforcement on /loaded-plugins; refactor to a shared buildApp helper.

Layer 2 (integration):
- licensed-users-info: new startTestBackend pilot that boots the real plugin
  against SQLite and hits /health. plugin.ts coverage 0 -> 100%, runs in ~3s.

Overall licensed-users-info coverage ~17% -> 86%. No cluster required.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 20, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.71%. Comparing base (b08abdf) to head (1fa927b).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4863       +/-   ##
===========================================
+ Coverage   41.03%   72.71%   +31.67%     
===========================================
  Files         121      120        -1     
  Lines        2220     4914     +2694     
  Branches      562      558        -4     
===========================================
+ Hits          911     3573     +2662     
- Misses       1304     1340       +36     
+ Partials        5        1        -4     
Flag Coverage Δ
install-dynamic-plugins 92.44% <ø> (?)
rhdh 48.64% <ø> (+7.60%) ⬆️
Components Coverage Δ
Backend plugins 100.00% <ø> (+43.52%) ⬆️
Backend app 64.63% <ø> (+39.10%) ⬆️
Frontend app 40.57% <ø> (ø)
Plugin utils ∅ <ø> (∅)
Dynamic plugins utils ∅ <ø> (∅)

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 b08abdf...1fa927b. 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.

Address review feedback on the Layer 1+2 backend tests:

- licensed-users-info: cover the SQLite in-memory auto-disable path
  (empty router + warn), the CSV conversion failure (500), and the
  no-catalog-match enrichment branch. Add Layer 1 unit tests for
  CatalogEntityStore (ref mapping/filtering) and DatabaseUserInfoStore
  (count + "no user info found" error). Add clamp boundary cases.
- dynamic-plugins-info: replace the DynamicPluginManager private-field
  hack with a stub against the public plugins() contract, cover the
  front-end plugin branch, and add a Layer 2 integration test that boots
  the real plugin via a mocked dynamicPluginsServiceRef.
- Dedup the permissionCheck setup with a small helper.

Both plugins now report 100% statement/branch/function/line coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@gustavolira gustavolira changed the title test(backend): Layer 1+2 backend plugin tests (RHIDP-13233) test(backend): add Layer 1+2 backend plugin tests (RHIDP-13233) May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Image was built and published successfully. It is available at:

gustavolira and others added 2 commits May 20, 2026 18:26
Expand Layer 1+2 coverage to the remaining backend code:

- scalprum-backend: add a Layer 2 integration test (boots the real plugin
  via a mocked dynamicPluginsServiceRef) and a unit test for the
  "no matching package" warn branch. Plugin now at 100% coverage.
- packages/backend auth resolvers (previously 0%): unit tests for
  resolverUtils (createOidcSubClaimResolver sub/id-token/mismatch paths,
  trySignInResolvers fallthrough) and rhdhSignInResolvers (preferred
  username, oauth2-proxy header precedence, LDAP uuid matching, Keycloak
  and Ping Identity sub-claim wiring, dangerous fallback options).

resolverUtils.ts, rhdhSignInResolvers.ts and scalprum router/plugin all
report 100% coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review feedback on the sign-in resolver tests:

- oauth2-proxy: split the fallback test into an explicit precedence case
  (preferred-username wins when both headers are present) and the
  x-forwarded-user fallback, so the header ordering is actually pinned.
- trySignInResolvers: assert each resolver is attempted exactly once for
  both the skip-then-succeed and all-fail paths, locking the iteration
  contract against short-circuit regressions.
- createOidcSubClaimResolver: cover an id token that carries no sub claim.
- Use `as unknown as` for the empty context/info doubles.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@gustavolira gustavolira changed the title test(backend): add Layer 1+2 backend plugin tests (RHIDP-13233) test(backend): add Layer 1+2 tests for backend plugins and auth resolvers (RHIDP-13233) May 20, 2026
A single startTestBackend smoke per plugin underused Layer 2. Add a
distinct end-to-end scenario to each so the real wiring (HTTP routing,
auth, config-driven init, static serving) is actually exercised:

- scalprum: boot a web plugin from a mock dist-scalprum directory and
  assert both the /plugins listing and the statically served manifest.
- dynamic-plugins-info: assert installer details are stripped over the
  real backend, and that service principals (not just users) are
  authorized to read the plugin list.
- licensed-users-info: assert the plugin disables all routes (404) when
  configured with a pure in-memory SQLite database, through real init.

All three plugins remain at 100% coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

pluginIDProviderService derives RBAC plugin ids from dynamic backend
packages via a regex that strips scope/plugin prefixes and the
-backend-dynamic suffix — previously untested. Add Layer 1 tests (via
ServiceFactoryTester) covering the core ids, name normalization across
@scope/plugin-, backstage-plugin- and bare prefix styles, and exclusion
of non-backend packages. The factory logic is now 100% branch-covered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

The comment layout already renders a `components` section but no
components were defined, so the dashboard and PR comment showed none.
Define path-based components for the main source areas (backend plugins,
backend app, frontend app, plugin-utils, dynamic-plugins utils) so
coverage is visible per area. Components are views over existing uploads,
so no CI changes are needed; statuses are informational while coverage
matures, matching the project/patch approach. Validated via codecov.io/validate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@gustavolira
Copy link
Copy Markdown
Member Author

/review
--pr_reviewer.inline_code_comments=true
-i
--pr_reviewer.require_score_review=true
--pr_reviewer.require_can_be_split_review=true
--pr_reviewer.num_code_suggestions="2"

@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 91
🧪 PR contains tests
🔒 No security concerns identified
🔀 Multiple PR themes

Sub-PR theme: codecov: add component management views

Relevant files:

  • codecov.yml

Sub-PR theme: test(backend): add unit tests for auth resolvers and dynamic plugin id provider

Relevant files:

  • packages/backend/src/modules/resolverUtils.test.ts
  • packages/backend/src/modules/rhdhSignInResolvers.test.ts
  • packages/backend/src/modules/rbacDynamicPluginsModule.test.ts

Sub-PR theme: test(plugins): add Layer 1/2 tests for backend plugins routers and stores

Relevant files:

  • plugins/dynamic-plugins-info-backend/src/service/router.test.ts
  • plugins/dynamic-plugins-info-backend/src/service/router.integration.test.ts
  • plugins/licensed-users-info-backend/src/service/router.test.ts
  • plugins/licensed-users-info-backend/src/service/router.integration.test.ts
  • plugins/licensed-users-info-backend/src/service/catalogStore.test.ts
  • plugins/licensed-users-info-backend/src/service/readBackstageTokenExpiration.test.ts
  • plugins/licensed-users-info-backend/src/database/databaseUserInfoStore.test.ts
  • plugins/scalprum-backend/src/service/router.test.ts
  • plugins/scalprum-backend/src/service/router.integration.test.ts

⚡ Recommended focus areas for review

HTTP Semantics

The CSV branch test triggers CSV output by setting content-type on a GET request, which is typically a request-body header and may not reflect real client behavior. Consider asserting CSV via the Accept header (and/or whatever header the router actually checks) to avoid a test that passes while clients still can't negotiate CSV correctly.

it('returns the list as CSV when the content-type is text/csv', async () => {
  mockGetListUsers.mockResolvedValue([userRow]);

  const response = await request(app)
    .get('/users')
    .set('content-type', 'text/csv');

  expect(response.status).toEqual(200);
  expect(response.headers['content-type']).toContain('text/csv');
  expect(response.text).toContain('userEntityRef');
  expect(response.text).toContain('user:default/jdoe');
});

it('returns users without profile data when no catalog entity matches', async () => {
  mockGetListUsers.mockResolvedValue([userRow]);
  mockGetUserEntities.mockResolvedValue(new Map());

  const response = await request(app).get('/users');

  expect(response.status).toEqual(200);
  expect(response.body[0]).toEqual({
    userEntityRef: 'user:default/jdoe',
    lastAuthTime: expect.any(String),
  });
});

it('returns 500 when CSV conversion fails', async () => {
  mockGetListUsers.mockResolvedValue([userRow]);
  (json2csv as jest.Mock).mockImplementationOnce(() => {
    throw new Error('conversion failed');
  });

  const response = await request(app)
    .get('/users')
    .set('content-type', 'text/csv');

  expect(response.status).toEqual(500);
  expect(response.text).toContain('Error converting to CSV');
});
Mock Robustness

The Knex mock relies on a thenable object that also has chainable methods. This is clever but can be brittle if the production code changes the chaining order or uses additional builder methods. Consider tightening the mock shape to exactly what the store calls (and/or adding an assertion that the expected query methods were invoked) so failures are clearer and less sensitive to promise/method interop quirks.

// Knex query builders are both thenable (resolve to rows) and chainable
// (`.count().first()`). The helper reproduces both shapes from a single call
// so the store can be unit-tested without a real database.
const mockDatabase = (rows: UserInfoRow[], countResult?: { count: number }) => {
  const query: any = Promise.resolve(rows);
  query.count = jest.fn().mockReturnValue({
    first: jest.fn().mockResolvedValue(countResult),
  });
  return jest.fn().mockReturnValue(query) as unknown as Knex;
};
📚 Focus areas based on broader codebase context

Cleanup

The integration test creates a temporary mock directory via createMockDirectory() but does not clear or dispose it after the test runs. Add an afterEach/afterAll cleanup (e.g., mockDir.clear() and any other teardown needed) to avoid cross-test contamination and leaking temp resources when the suite grows. (Ref 8)

it('lists a web plugin and serves its manifest as static content', async () => {
  const mockDir = createMockDirectory();
  mockDir.setContent({
    'dist-scalprum': {
      'plugin-manifest.json': JSON.stringify({ name: 'scalprum-plugin' }),
    },
  });

Reference reasoning: Existing tests that use createMockDirectory() include explicit teardown steps (clearing the directory and resetting state) to keep tests isolated and prevent resource leakage across cases. Adopting the same pattern here keeps the Layer 2 integration tests consistent and stable over time.

📄 References
  1. redhat-developer/rhdh/e2e-tests/playwright/e2e/plugins/licensed-users-info-backend/licensed-users-info.spec.ts [1-135]
  2. redhat-developer/rhdh/packages/backend/src/modules/rhdhSignInResolvers.ts [1-31]
  3. redhat-developer/rhdh/packages/backend/src/modules/rhdhSignInResolvers.ts [32-167]
  4. redhat-developer/rhdh/e2e-tests/playwright/e2e/auth-providers/oidc.spec.ts [1-19]
  5. redhat-developer/rhdh/e2e-tests/playwright/e2e/auth-providers/oidc.spec.ts [21-536]
  6. redhat-developer/rhdh/packages/backend/src/index.ts [132-163]
  7. redhat-developer/rhdh/packages/backend/src/modules/authProvidersModule.ts [47-70]
  8. redhat-developer/rhdh/e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-rbac.spec.ts [14-1570]

- scalprum integration test: create the mock directory at describe scope
  so its afterAll cleanup is registered and the temp dir does not leak
  across tests (Qodo: test isolation).
- databaseUserInfoStore: expose the query spies from the Knex mock and
  assert the store calls the user_info table and the count query with
  the expected alias, so failures are clearer and less sensitive to the
  thenable/chainable interop (Qodo: mock robustness).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown
Contributor

Image was built and published successfully. It is available at:

@gustavolira gustavolira marked this pull request as ready for review May 21, 2026 14:34
@openshift-ci openshift-ci Bot requested review from hopehadfield and rm3l May 21, 2026 14:34
@github-actions
Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days.

@github-actions github-actions Bot added Stale and removed Stale labels May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant