Skip to content

test(frontend): close the user, hub and resource-registry service gaps - #8336

Merged
aglinxinyuan merged 4 commits into
apache:mainfrom
aglinxinyuan:cov/frontend-user-and-hub-services
Sep 2, 2026
Merged

test(frontend): close the user, hub and resource-registry service gaps#8336
aglinxinyuan merged 4 commits into
apache:mainfrom
aglinxinyuan:cov/frontend-user-and-hub-services

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Four existing frontend specs extended. +10 fully-covered lines and +7 branch arms.

File Codecov Branch arms
user.service.ts 62/66 → 66/66 19/21 → 21/21
browse-section.component.ts 28/32 → 31/32 22/25 → 24/25
resource-registry.service.ts 18/20 → 20/20 16/17 → 17/17
email-request-modal.component.html 14/15 → 15/15 4/6 → 6/6

Three of the four reach 100%. The plain line-hit metric moves only +4 against Codecov's +10, because six of the gained lines were already executing and flip solely by completing a branch arm — the two numbers are not interchangeable and both are given.

Not-fully-covered across the bundle goes 13 → 3, and all three of those are deliberately declined (below).

The reviewer revised my own claim down

The build reported +11 lines and +8 arms. Independent re-measurement puts it at +10 and +7: browse-section.component.ts gains 3 lines and 2 arms, not 4 and 3. Line 96 remains partial. The lower figure is the one in the table.

Two files were in scope and contribute nothing

  • user-dataset-version-creator.component.ts was dropped entirely. Its only missed line is get formControlNames(), and a repo-wide grep across .ts and .html returns exactly one hit — its own declaration. Zero call sites, zero template bindings. A test there would be a pure count-raiser, so the getter is flagged as a dead-code removal candidate instead.
  • browse-section.component.html (24/24) and email-request-modal.component.ts (8/9) were measured both ways and are unchanged.

Deliberately not included

browse-section.component.ts:96 stays partial, and email-request-modal.component.ts:67 is declined.

One survivor is reported rather than chased: mutating user.service.ts:108 from handleAccessToken(accessToken ?? "") to a conditional survives the suite. That mutant is character-for-character the shape register() already uses for the same string | null payload — it is arguably the fix, not a regression, so no test was written to pin the current form.

user.service.ts:184 (this.cache.delete(avatarUrl)) is covered but not independently pinned: the code immediately falls through to a map that overwrites the entry either way, so no mutation isolates it. It rides along with lines 180 and 183 in one test, and that is stated rather than presented as a kill.

Verification

Measured with the full 209-file suite in one command — no name filter and no --include at all — so there is no filter-attribution risk, with coverage/ deleted before each run. The baseline was rebuilt by writing the HEAD versions of the four specs into place from a scratch extraction (never git checkout), running, then restoring from a hash-verified snapshot. Figures parsed straight from coverage/gui/lcov.info.

Seven reviewer findings, all repaired — including one test that was deleted rather than kept, because it duplicated an existing assertion.

browse-section.component.ts:114 needed a white-box assertion or the test would have been vacuous: getCoverImage()'s || this.defaultBackground makes the guarded and unguarded paths observably identical through the public API, so the test asserts on the private coverImageUrls map. That is unusual and is called out here rather than left for a reader to find.

yarn format:ci passes. frontend/junit.xml is regenerated by every run, is not gitignored, and is not committed. No production file is touched.

Any related issues, documentation, discussions?

Closes #8334

How was this PR tested?

npx ng test --watch=false --include="**/user.service.spec.ts" --include="**/browse-section.component.spec.ts" --include="**/resource-registry.service.spec.ts" --include="**/email-request-modal.component.spec.ts"
 Test Files  4 passed (4)

Re-run after rebasing onto current main.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

Copilot AI lite review requested due to automatic review settings September 2, 2026 02:26
@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Neilk1021, @tanishqgandhi1908
    You can notify them by mentioning @Neilk1021, @tanishqgandhi1908 in a comment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes are limited to test-only additions that align with existing production behavior and improve branch coverage without introducing functional risk.

Pull request overview

This PR extends the Angular/Vitest frontend unit test suite to close remaining coverage gaps called out in #8334, focusing on previously unexercised guard branches and template binding write-backs without changing production code.

Changes:

  • Adds BrowseSectionComponent spec coverage for cover-image caching behavior and guard paths when a descriptor lacks coverUrl or returns an empty URL.
  • Extends ResourceRegistryService specs to cover the hubRoute-absent branch (private-page-only kinds).
  • Extends UserService and EmailRequestModalComponent specs to cover additional edge paths (null access token on verify, avatar cache expiry revoke/delete path, ngModel write-back for code input).
File summaries
File Description
frontend/src/app/hub/component/browse-section/browse-section.component.spec.ts Adds tests covering cover image cache guards, empty-cover handling, and avoiding duplicate descriptor calls across ngOnChanges.
frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.spec.ts Adds a test covering entryLink behavior when a descriptor has a privateRoute but no hubRoute.
frontend/src/app/common/service/user/user.service.spec.ts Adds tests covering register-verify null-token behavior and avatar cache expiry behavior (revoke/delete + refetch).
frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.spec.ts Adds a test that types into the verification-code input and asserts ngModel write-back and correct label content.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

codecov-commenter commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.17%. Comparing base (464aa8c) to head (3098472).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8336      +/-   ##
============================================
+ Coverage     94.08%   94.17%   +0.08%     
+ Complexity     4810     4761      -49     
============================================
  Files          1197     1192       -5     
  Lines         48811    48520     -291     
  Branches       5906     5417     -489     
============================================
- Hits          45925    45692     -233     
+ Misses         1427     1418       -9     
+ Partials       1459     1410      -49     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 98588bf
agent-service 99.32% <ø> (-0.01%) ⬇️ Carriedforward from 98588bf
amber 90.03% <ø> (-0.03%) ⬇️ Carriedforward from 98588bf
computing-unit-managing-service 73.67% <ø> (ø) Carriedforward from 98588bf
config-service 86.73% <ø> (-0.27%) ⬇️ Carriedforward from 98588bf
file-service 87.91% <ø> (ø) Carriedforward from 98588bf
frontend 96.87% <ø> (+0.04%) ⬆️
notebook-migration-service 79.31% <ø> (ø) Carriedforward from 98588bf
pyamber 98.99% <ø> (+0.52%) ⬆️ Carriedforward from 98588bf
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 98588bf

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@aglinxinyuan
aglinxinyuan requested a review from mengw15 September 2, 2026 03:36
@mengw15
mengw15 requested a lite review from Copilot September 2, 2026 06:45

@mengw15 mengw15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Changes are test-only coverage improvements with minor follow-up nits that don’t materially affect correctness or stability.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Sep 2, 2026
Merged via the queue into apache:main with commit 30681cd Sep 2, 2026
22 checks passed
@aglinxinyuan
aglinxinyuan deleted the cov/frontend-user-and-hub-services branch September 2, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit test coverage for the user, hub and resource-registry services

4 participants