Skip to content

test(computing-unit): extend ComputingUnitManagingResourceSpec to cover the create, rename and configuration endpoints - #7580

Merged
aglinxinyuan merged 1 commit into
apache:mainfrom
eugenegujing:test/computing-unit-resource-endpoints
Aug 13, 2026
Merged

test(computing-unit): extend ComputingUnitManagingResourceSpec to cover the create, rename and configuration endpoints#7580
aglinxinyuan merged 1 commit into
apache:mainfrom
eugenegujing:test/computing-unit-resource-endpoints

Conversation

@eugenegujing

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

This PR extends ComputingUnitManagingResourceSpec (added in #6853, extended in #7337) to the endpoints of ComputingUnitManagingResource that had no coverage. The existing spec only covered getComputingUnitInfo, getComputingUnitMetricsEndpoint, listComputingUnits and terminateComputingUnit; the create, rename and configuration endpoints were untested.

All new tests keep the existing spec's approach: local-type units driven against the embedded Postgres (MockTexeraDB), so no Kubernetes calls are made. New coverage, by endpoint:

  • createWorkflowComputingUnit — local happy path (persisted with a generated cuid, the user URI landing in both the uri column and the resource JSON's nodeAddresses, response reporting owner/WRITE/Running with NaN metrics); whitespace-only name rejected with ForbiddenException and nothing stored; unknown type (quantum) rejected; kubernetes type rejected while disabled; missing and blank URI rejected; the per-user running-unit quota not applying to local units.
  • renameComputingUnit — owner success; non-owner without access 403 (name kept); READ-only grantee 403; WRITE grantee success; blank name 400; nonexistent unit NotFoundException; database failure (name overflowing the VARCHAR(128) column) rolling back and keeping the name; and an admin who neither owns nor was granted access getting 403 (rename, unlike terminate, has no ADMIN bypass).
  • terminateComputingUnit — a WRITE grantee rejected with 400 and the unit not terminated (terminate requires strict ownership or the ADMIN role).
  • getComputingUnitInfo — a READ grantee sees the unit with isOwner = false and accessPrivilege = READ; a nonexistent unit yields NotFoundException; the owner response also reports the non-empty owner avatar.
  • getComputingUnitTypes — lists exactly local while Kubernetes is disabled.
  • getComputingUnitLimitOptions — returns the configured cpu/memory/gpu option lists.
  • getComputingUnitResourceLimit — the local branch returns NaN limits for the owner; a non-owner gets BadRequestException; a nonexistent unit yields NotFoundException.
  • getComputingUnitMetricsEndpoint — adds the missing negative direction: a non-owner gets BadRequestException.

Note: the Kubernetes-only validation in createWorkflowComputingUnit sits behind the supported-type gate and is unreachable while kubernetes.enabled is false; that flag is a load-time val the test JVM does not override, so those branches cannot be exercised in this suite. A spec comment records this.

No production code is changed.

Any related issues, documentation, discussions?

Closes #7576

How was this PR tested?

This PR is itself test-only. The new specs were run with:

sbt "ComputingUnitManagingService/testOnly org.apache.texera.service.resource.ComputingUnitManagingResourceSpec"

All 31 tests pass (7 pre-existing + 24 new) against the embedded database; no external services are needed. The suite was mutation-checked: targeted mutations of the resource (removing the blank-name and missing-URI checks, inverting the rename ownership gate, skipping the rename blank-name 400, making getComputingUnitTypes also return kubernetes, and removing the non-owner check in getComputingUnitResourceLimit) each caused at least one new test to fail, and the source was restored afterwards. ComputingUnitManagingService/Test/scalafmtCheck passes.

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

Co-authored by: Claude Code (Claude Fable 5)

…oints

Extend ComputingUnitManagingResourceSpec to the createWorkflowComputingUnit, renameComputingUnit, getComputingUnitTypes, getComputingUnitLimitOptions and getComputingUnitResourceLimit endpoints, using local-type units against the embedded database.
@github-actions github-actions Bot added the platform Non-amber Scala service paths label Aug 11, 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:

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

@eugenegujing eugenegujing changed the title test(computing-unit): cover the create, rename and configuration endpoints test(computing-unit): extend ComputingUnitManagingResourceSpec to cover the create, rename and configuration endpoints Aug 11, 2026
@codecov-commenter

codecov-commenter commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.99%. Comparing base (e878df3) to head (3900c9d).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7580      +/-   ##
============================================
+ Coverage     87.88%   87.99%   +0.10%     
- Complexity     4276     4292      +16     
============================================
  Files          1176     1176              
  Lines         47018    47018              
  Branches       5245     5245              
============================================
+ Hits          41323    41373      +50     
+ Misses         3972     3920      -52     
- Partials       1723     1725       +2     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 98.62% <ø> (ø) Carriedforward from e878df3
amber 82.88% <ø> (ø) Carriedforward from e878df3
computing-unit-managing-service 72.46% <ø> (+12.07%) ⬆️
config-service 65.97% <ø> (ø)
file-service 69.05% <ø> (ø)
frontend 89.94% <ø> (ø) Carriedforward from e878df3
notebook-migration-service 78.89% <ø> (ø)
pyamber 97.52% <ø> (ø) Carriedforward from e878df3
workflow-compiling-service 26.31% <ø> (ø)

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

@eugenegujing

Copy link
Copy Markdown
Contributor Author

/request-review @aglinxinyuan

@github-actions
github-actions Bot requested a review from aglinxinyuan August 13, 2026 03:55
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 13, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 13, 2026
Merged via the queue into apache:main with commit c7e7362 Aug 13, 2026
37 checks passed
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Aug 13, 2026
…ache#7633)

### What changes were proposed in this PR?

**`main` does not compile.** Any PR whose `build / amber` or `build /
amber-integration` jobs run after this landed fails on it regardless of
what the PR itself touches — apache#7631 is an example, where the only change
is four tests in an unrelated module's spec.

Scoping that honestly: PRs whose amber jobs ran *before* the breakage
still show green and would fail on re-run, and frontend-labelled PRs
skip the amber stack, so this is "every amber run from now until it is
fixed" rather than "every open PR is red today".

`ComputingUnitManagingResourceSpec` asserts on
`DashboardWorkflowComputingUnit.ownerGoogleAvatar`, but the field is
named `ownerAvatar`, so `ComputingUnitManagingService / Test` fails with
two "value ownerGoogleAvatar is not a member" errors.

Two PRs raced to produce it: apache#7563 renamed the field to `ownerAvatar`,
while apache#7580 added assertions written against the old name. Each was
green against its own base, and the combination is what breaks — the
kind of thing per-PR CI cannot see when two PRs touch different files.

This renames the two accessor calls. Nothing else changes.

### How was this PR tested?

Confirmed the breakage is real and that this is the whole of it, by
stashing the change and re-running on otherwise-clean `main`:

```
sbt "ComputingUnitManagingService/Test/compile"
```

| | Result |
|---|---|
| unpatched `main` | exit 1, exactly 2 × `value ownerGoogleAvatar is not
a member` |
| with this change | exit 0, compiles clean |

Then the spec itself:

```
sbt "ComputingUnitManagingService/testOnly org.apache.texera.service.resource.ComputingUnitManagingResourceSpec"
```

```
[info] Total number of tests run: 31
[info] Tests: succeeded 31, failed 0, canceled 0, ignored 0, pending 0
```

All 31 pass, so the assertions were correct about the value and only the
accessor name was stale. `Test/scalafmtCheck` and `Test/scalafix
--check` both pass.

### Any related issues, documentation, discussions?

Closes apache#7632

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

Generated-by: Claude Code (Opus 5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform Non-amber Scala service paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extend ComputingUnitManagingResourceSpec to cover the create, rename, and configuration endpoints

3 participants