Skip to content

test(frontend): add unit tests for AdminUserService#6467

Open
Ma77Ball wants to merge 3 commits into
apache:mainfrom
Ma77Ball:test/admin-user-service-spec
Open

test(frontend): add unit tests for AdminUserService#6467
Ma77Ball wants to merge 3 commits into
apache:mainfrom
Ma77Ball:test/admin-user-service-spec

Conversation

@Ma77Ball

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

  • Adds admin-user.service.spec.ts covering AdminUserService; no production code changed.
  • Verifies the user list/update/add endpoints and the per-user quota GET endpoints.
  • Asserts which endpoints send the user_id query param and which do not.

Any related issues, documentation, discussions?

Closes: #6455

How was this PR tested?

  • Run cd frontend && npx nx test gui --include="**/admin-user.service.spec.ts", expect 11 passed.
  • Full frontend suite runs in CI via yarn test:ci, selected by the auto-applied frontend label.

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

Co-authored with Claude Opus 4.8 in compliance with ASF

@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • No candidates found from git blame history.

@codecov-commenter

codecov-commenter commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.27%. Comparing base (c7473a1) to head (f3b1815).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6467      +/-   ##
============================================
+ Coverage     70.20%   70.27%   +0.06%     
  Complexity     3390     3390              
============================================
  Files          1142     1142              
  Lines         44844    44871      +27     
  Branches       4949     4955       +6     
============================================
+ Hits          31484    31534      +50     
+ Misses        11725    11701      -24     
- Partials       1635     1636       +1     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 5cffe2b
agent-service 76.76% <ø> (ø) Carriedforward from 5cffe2b
amber 66.68% <ø> (ø) Carriedforward from 5cffe2b
computing-unit-managing-service 9.97% <ø> (ø) Carriedforward from 5cffe2b
config-service 52.30% <ø> (ø) Carriedforward from 5cffe2b
file-service 65.63% <ø> (ø) Carriedforward from 5cffe2b
frontend 69.14% <ø> (+0.17%) ⬆️
notebook-migration-service 78.94% <ø> (ø) Carriedforward from 5cffe2b
pyamber 91.19% <ø> (ø) Carriedforward from 5cffe2b
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 5cffe2b

*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.

@Ma77Ball

Copy link
Copy Markdown
Contributor Author

/request-review @mengw15

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.

Pull request overview

Adds a new frontend unit test spec to cover AdminUserService’s HTTP wiring (URLs, methods, request bodies, and user_id query param usage) using Angular’s HttpClientTestingModule.

Changes:

  • Added admin-user.service.spec.ts with request-shape assertions for the user list/update/add endpoints.
  • Added request-shape assertions for per-user “quota/statistics” GET endpoints (including which ones include user_id as a query param).
  • Added a DELETE test for the per-execution collection endpoint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@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, left one note

req.flush({});
});

it("getUploadedFiles() sends the uid as a user_id query param", () => {

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.

Not blocking — a note on the three methods this and the two tests below pin: getUploadedFiles, getCreatedDatasets, and getAccessFiles are orphaned dead code calling endpoints that no longer exist. AdminUserResource (/admin/user) only defines list / update / add / created_workflows / access_workflows / user_quota_size / deleteCollection; a repo-wide search finds no uploaded_files or access_files route anywhere (created_datasets exists only under the session-scoped /quota resource); and no frontend component calls any of the three — calling them would 404. They were live once, but #2781 (Remove File-Related Components) deleted their callers and backend routes and left the service methods behind. Since HttpTestingController intercepts everything, these tests stay green while the methods themselves are unusable.

Same situation as #6289/#6305: consider dropping these three tests and deleting the three methods (+ URL constants) in a follow-up instead of cementing them. The other eight tests all target real routes and look good.

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 frontend unit tests for AdminUserService

4 participants