Skip to content

test(app): raise packages/app UI coverage with component and unit tests (RHIDP-13853)#4865

Open
gustavolira wants to merge 4 commits into
redhat-developer:mainfrom
gustavolira:RHIDP-13853-app-ui-coverage
Open

test(app): raise packages/app UI coverage with component and unit tests (RHIDP-13853)#4865
gustavolira wants to merge 4 commits into
redhat-developer:mainfrom
gustavolira:RHIDP-13853-app-ui-coverage

Conversation

@gustavolira
Copy link
Copy Markdown
Member

@gustavolira gustavolira commented May 20, 2026

Description

Implements RHIDP-13853 — raises packages/app UI coverage by adding Layer 3 component/unit tests for previously-untested modules that carried the most uncovered lines (identified via Codecov). Complements RHIDP-13235 (which only covers behaviors mirrored by existing UI E2E specs).

Tests added (target → coverage)

Module Before After (lines)
api/LearningPathApiClient.ts 13% 100%
components/Root/MenuIcon.tsx 21% 100%
components/ErrorPages/errorButtons/GoBackButton.tsx 0% 100%
components/ErrorPages/errorButtons/ContactSupportButton.tsx 0% 100%
components/ErrorPages/ErrorPage.tsx 0% 100%
components/SignInPage/SignInPage.tsx 0% 100%
components/Root/ResizableDrawer.tsx 12% ~85%

What the tests assert:

  • LearningPathApiClient — URL building (default + configured proxy path), bearer-token header presence/absence, non-ok error path.
  • MenuIcon — empty / registered system icon / inline-SVG / URL / material-name branches.
  • GoBackButton — navigates back when history exists, renders nothing otherwise.
  • ContactSupportButton — prop > configured URL > default fallback.
  • ErrorPage — status/message/info, contact-support action, stack trace, no go-back for non-404.
  • ResizableDrawer — children render while open, resize listeners registered when resizable, resize drag reports the new width.
  • SignInPage — proxied vs local provider selection, guest only in development, default-to-github when unset/unknown (core-components widgets stubbed to isolate the selection logic).

Out of scope

  • Root.tsx (~130 uncovered lines) is the largest remaining target but high-effort (full sidebar nav) — left as a follow-up.
  • DynamicRoot.tsx reports 0% despite having a test (lazy-import coverage-instrumentation artifact) — separate investigation.
  • Entrypoints, trivial icon exports, and apis.ts factory wiring — low value.

Testing

  • 25 tests pass across 7 suites
  • tsc, lint and prettier pass
  • New tests contribute to the Codecov rhdh flag

gustavolira and others added 2 commits May 20, 2026 20:52
Net-new unit/component tests for previously-untested packages/app modules
(RHIDP-13853, UI coverage):

- LearningPathApiClient: URL building (default + configured proxy path),
  bearer-token header presence/absence, and the non-ok error path.
- MenuIcon: empty icon, registered system icon, inline SVG (base64 image),
  URL icon, and material icon name branches.
- GoBackButton: navigates back when history exists, renders nothing otherwise.
- ContactSupportButton: prop > configured URL > default fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Net-new component tests for previously-untested packages/app UI
(RHIDP-13853):

- ErrorPage: status/message/info rendering, the always-present
  contact-support action, stack-trace rendering, and no go-back action
  for non-404 errors.
- ResizableDrawer: children render while open, resize listeners are
  registered when resizable, and a resize drag reports the new width.
- SignInPage: proxied vs local provider selection, guest provider added
  in development only, default-to-github when unset or unknown.

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 21, 2026

Codecov Report

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

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4865       +/-   ##
===========================================
+ Coverage   41.03%   72.67%   +31.63%     
===========================================
  Files         121      111       -10     
  Lines        2220     4724     +2504     
  Branches      562      535       -27     
===========================================
+ Hits          911     3433     +2522     
+ Misses       1304     1291       -13     
+ Partials        5        0        -5     
Flag Coverage Δ
install-dynamic-plugins 92.44% <ø> (?)
rhdh 46.29% <ø> (+5.25%) ⬆️

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...9ae2bae. 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.

- ErrorPage: assert the go-back action appears for 404 errors when there
  is history (the 404 branch), bringing the file to 100% coverage.
- ResizableDrawer: assert a mouse-up ends the drag so a later mouse-move
  is ignored, covering the mouse-up handler.

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.

- ResizableDrawer: assert the resize handle is found before dispatching
  (so the mouse-up test cannot pass for the wrong reason), and add a
  rerender-based test for the re-clamp-below-minimum branch — the file is
  now 100% line-covered.
- ErrorPage / GoBackButton: restore window.history.length after each test
  so the history-dependent cases are reorder-safe.
- GoBackButton: drop the redundant MemoryRouter wrapper (useNavigate is
  mocked).

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:35
@openshift-ci openshift-ci Bot requested review from PatAKnight and kadel May 21, 2026 14:35
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to tests for button that just calls navigate(-1);?

This feels like writing tests only for sake of writing tests and increasing coverage numbers rather than providing actual value.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was already written so we can keep it. But I don't think that it makes much sense to test functions like this.
ContactSupportButton and ErrorPage are similar stories.

Copy link
Copy Markdown
Member Author

@gustavolira gustavolira May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to tests for button that just calls navigate(-1);?

This feels like writing tests only for sake of writing tests and increasing coverage numbers rather than providing actual value.

The real value there is the conditional rendering, not the click itself

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was already written so we can keep it. But I don't think that it makes much sense to test functions like this. ContactSupportButton and ErrorPage are similar stories.

Agree they're simple components, but ContactSupportButton has a 3-level fallback (prop, config, default URL) that can break silently, and ErrorPage has conditional logic worth covering. Not strictly required, but I think they add some value.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants