Skip to content

test(amber): add unit test coverage for AdminExecutionResource - #7165

Merged
mengw15 merged 2 commits into
apache:mainfrom
mengw15:chore/7160-admin-execution-test
Jul 31, 2026
Merged

test(amber): add unit test coverage for AdminExecutionResource#7165
mengw15 merged 2 commits into
apache:mainfrom
mengw15:chore/7160-admin-execution-test

Conversation

@mengw15

@mengw15 mengw15 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds a spec for AdminExecutionResource
(amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResource.scala),
which had no test coverage. The resource is jOOQ-backed, so the spec runs against
embedded Postgres via MockTexeraDB (the established unit pattern, mirroring
WorkflowExecutionsResourceSpec). No production code was changed.

8 tests:

  • mapToName / mapToStatus — the pure status-code lookups, covering every known
    code and the default ("UNKNOWN" / -1); no DB needed.
  • getTotalWorkflows — returns 0 with no executions, and counts distinct
    workflows (two executions on one workflow still count as one).
  • listWorkflows — seeds a user, workflow, version and executions, then asserts:
    the returned row's fields (name, owner, mapped status, execution name); the
    access flag derived from WORKFLOW_USER_ACCESS (true with a grant, false
    without); that only the latest execution per workflow is returned; that the
    status filter narrows the result; and that providing a sort field still returns
    the row.

Any related issues, documentation, discussions?

Closes #7160

How was this PR tested?

Unit tests, run locally against embedded Postgres (MockTexeraDB). All pass, and
the failure path was verified by breaking an assertion to confirm the suite goes
red:

sbt "WorkflowExecutionService/testOnly *AdminExecutionResourceSpec"
# Tests: succeeded 8, failed 0
sbt "WorkflowExecutionService/scalafmtCheck"      # clean
sbt "WorkflowExecutionService/Test/scalafix --check"   # clean

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

Generated-by: Claude Code (Opus 4.8 [1M context])

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

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 ScalaTest unit coverage for the Amber admin execution dashboard resource (AdminExecutionResource) using the existing MockTexeraDB embedded-Postgres pattern, without modifying production code.

Changes:

  • Introduces AdminExecutionResourceSpec to exercise the pure status-code mapping helpers (mapToName, mapToStatus).
  • Adds DB-backed tests for getTotalWorkflows and listWorkflows (access flag, latest execution selection, status filtering, and a basic sorting invocation).

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

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 7 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 45b7b20 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 397 0.242 23,949/36,925/36,925 us 🔴 +18.1% / 🔴 +133.9%
🔴 bs=100 sw=10 sl=64 933 0.569 106,094/124,791/124,791 us 🔴 +5.8% / 🔴 +13.1%
bs=1000 sw=10 sl=64 1,106 0.675 904,075/947,673/947,673 us ⚪ within ±5% / 🟢 -11.8%
Baseline details

Latest main 45b7b20 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 397 tuples/sec 449 tuples/sec 771.62 tuples/sec -11.6% -48.5%
bs=10 sw=10 sl=64 MB/s 0.242 MB/s 0.274 MB/s 0.471 MB/s -11.7% -48.6%
bs=10 sw=10 sl=64 p50 23,949 us 22,342 us 12,626 us +7.2% +89.7%
bs=10 sw=10 sl=64 p95 36,925 us 31,270 us 15,786 us +18.1% +133.9%
bs=10 sw=10 sl=64 p99 36,925 us 31,270 us 19,344 us +18.1% +90.9%
bs=100 sw=10 sl=64 throughput 933 tuples/sec 982 tuples/sec 971.56 tuples/sec -5.0% -4.0%
bs=100 sw=10 sl=64 MB/s 0.569 MB/s 0.6 MB/s 0.593 MB/s -5.2% -4.0%
bs=100 sw=10 sl=64 p50 106,094 us 100,310 us 103,463 us +5.8% +2.5%
bs=100 sw=10 sl=64 p95 124,791 us 123,304 us 110,296 us +1.2% +13.1%
bs=100 sw=10 sl=64 p99 124,791 us 123,304 us 118,690 us +1.2% +5.1%
bs=1000 sw=10 sl=64 throughput 1,106 tuples/sec 1,106 tuples/sec 1,001 tuples/sec 0.0% +10.5%
bs=1000 sw=10 sl=64 MB/s 0.675 MB/s 0.675 MB/s 0.611 MB/s 0.0% +10.4%
bs=1000 sw=10 sl=64 p50 904,075 us 901,045 us 1,008,988 us +0.3% -10.4%
bs=1000 sw=10 sl=64 p95 947,673 us 964,966 us 1,055,260 us -1.8% -10.2%
bs=1000 sw=10 sl=64 p99 947,673 us 964,966 us 1,074,689 us -1.8% -11.8%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,503.58,200,128000,397,0.242,23948.95,36925.35,36925.35
1,100,10,64,20,2144.42,2000,1280000,933,0.569,106094.16,124790.70,124790.70
2,1000,10,64,20,18087.00,20000,12800000,1106,0.675,904075.24,947673.27,947673.27

@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.78%. Comparing base (45b7b20) to head (5c8beba).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7165      +/-   ##
============================================
+ Coverage     79.62%   79.78%   +0.16%     
- Complexity     3836     3838       +2     
============================================
  Files          1160     1160              
  Lines         46188    46188              
  Branches       5145     5145              
============================================
+ Hits          36777    36852      +75     
+ Misses         7778     7691      -87     
- Partials       1633     1645      +12     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from fbf0abb
agent-service 77.42% <ø> (ø) Carriedforward from fbf0abb
amber 73.53% <ø> (+0.44%) ⬆️
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from fbf0abb
config-service 65.97% <ø> (ø) Carriedforward from fbf0abb
file-service 66.80% <ø> (ø) Carriedforward from fbf0abb
frontend 83.16% <ø> (ø) Carriedforward from fbf0abb
notebook-migration-service 78.89% <ø> (ø) Carriedforward from fbf0abb
pyamber 97.36% <ø> (ø) Carriedforward from fbf0abb
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from fbf0abb

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

- Reword the random-id comment: MockTexeraDB's embedded Postgres is a JVM
  singleton and suites run sequentially, so the ids guard against cross-suite
  data residue, not parallel execution.
- Strengthen the sorting test to seed a second workflow with a distinct end
  time and assert the ascending/descending order, instead of only asserting
  the call does not throw.
@mengw15
mengw15 requested a review from aglinxinyuan July 31, 2026 12:59

@aglinxinyuan aglinxinyuan 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!

@mengw15
mengw15 added this pull request to the merge queue Jul 31, 2026
Merged via the queue into apache:main with commit 0022e35 Jul 31, 2026
40 of 45 checks passed
@mengw15
mengw15 deleted the chore/7160-admin-execution-test branch July 31, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add unit test coverage for AdminExecutionResource

4 participants