Skip to content

test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector#6822

Open
aglinxinyuan wants to merge 2 commits into
apache:mainfrom
aglinxinyuan:test-pybuilder-cov
Open

test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector#6822
aglinxinyuan wants to merge 2 commits into
apache:mainfrom
aglinxinyuan:test-pybuilder-cov

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds unit coverage for the two macro-only pybuilder helpers. Both classes run only during macro expansion, so the tests drive real expansions through a runtime ToolBox (the pattern the existing suite already uses) and assert on the captured expansion errors/behavior.

  • BoundaryValidatorSpec (+17): the validateCompileTime abort branches (splice inside an unclosed quote, after a # comment, bad left/right neighbor) and pass-through cases, plus the runtimeChecksForNestedBuilder guard (throws only for Encodable content in an unsafe context).
  • EncodableInspectorSpec (new, 12): Encodable detection via TYPE_USE annotation, symbol annotation, case-class accessor hop, and annotated return type; plus the non-Encodable cases and the nested-builder short-circuit.

No source files changed.

Any related issues, documentation, discussions?

Closes #6820.

How was this PR tested?

sbt -java-home <jbr-17> "PyBuilder/testOnly *BoundaryValidatorSpec *EncodableInspectorSpec" -> 32 succeeded, 0 failed. scalafmtCheckAll + scalafixAll --check clean.

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

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

…odableInspector

Both are macro-expansion-only helpers, so drive real expansions through a runtime
ToolBox (the pattern the existing suite uses). Extend BoundaryValidatorSpec (+17)
with the compile-time abort branches (unclosed-quote/after-comment/bad-neighbor) and
the runtime nested-builder guard, and add EncodableInspectorSpec (12) covering the
TYPE_USE / symbol-annotation / accessor-hop / annotated-return Encodable-detection
paths and the non-Encodable cases.
Copilot AI review requested due to automatic review settings July 23, 2026 06:32
@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: @kunwp1
    You can notify them by mentioning @kunwp1 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.

Pull request overview

Adds ScalaTest unit coverage for pybuilder’s macro-only helper logic by driving real pyb"..." macro expansions through a runtime ToolBox, allowing Jacoco to observe branches that normally only run at compile time.

Changes:

  • Add BoundaryValidatorSpec coverage for compile-time abort reasons, safe pass-through cases, and the nested-builder deferred runtime guard behavior.
  • Add new EncodableInspectorSpec covering Encodable detection paths (TYPE_USE annotation, symbol annotation, accessor hop, annotated return type) plus non-Encodable and nested-builder short-circuit cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/EncodableInspectorSpec.scala New ToolBox-driven spec to cover EncodableInspector’s Encodable classification branches and wrap/lowering behavior.
common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/BoundaryValidatorSpec.scala Expanded spec to cover BoundaryValidator compile-time abort branches and runtime guard behavior for nested builders.

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 0ba5d1a 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 404 0.246 24,407/30,108/30,108 us 🔴 +14.0% / 🔴 +91.0%
🔴 bs=100 sw=10 sl=64 808 0.493 123,646/152,476/152,476 us 🔴 +7.5% / 🔴 +42.0%
bs=1000 sw=10 sl=64 933 0.57 1,066,640/1,147,317/1,147,317 us ⚪ within ±5% / 🔴 +11.4%
Baseline details

Latest main 0ba5d1a from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 404 tuples/sec 448 tuples/sec 760.2 tuples/sec -9.8% -46.9%
bs=10 sw=10 sl=64 MB/s 0.246 MB/s 0.273 MB/s 0.464 MB/s -9.9% -47.0%
bs=10 sw=10 sl=64 p50 24,407 us 21,406 us 12,781 us +14.0% +91.0%
bs=10 sw=10 sl=64 p95 30,108 us 31,860 us 16,118 us -5.5% +86.8%
bs=10 sw=10 sl=64 p99 30,108 us 31,860 us 19,074 us -5.5% +57.9%
bs=100 sw=10 sl=64 throughput 808 tuples/sec 832 tuples/sec 988.68 tuples/sec -2.9% -18.3%
bs=100 sw=10 sl=64 MB/s 0.493 MB/s 0.508 MB/s 0.603 MB/s -3.0% -18.3%
bs=100 sw=10 sl=64 p50 123,646 us 119,272 us 101,448 us +3.7% +21.9%
bs=100 sw=10 sl=64 p95 152,476 us 141,864 us 107,410 us +7.5% +42.0%
bs=100 sw=10 sl=64 p99 152,476 us 141,864 us 120,258 us +7.5% +26.8%
bs=1000 sw=10 sl=64 throughput 933 tuples/sec 944 tuples/sec 1,023 tuples/sec -1.2% -8.8%
bs=1000 sw=10 sl=64 MB/s 0.57 MB/s 0.576 MB/s 0.624 MB/s -1.0% -8.7%
bs=1000 sw=10 sl=64 p50 1,066,640 us 1,064,518 us 983,888 us +0.2% +8.4%
bs=1000 sw=10 sl=64 p95 1,147,317 us 1,126,141 us 1,030,100 us +1.9% +11.4%
bs=1000 sw=10 sl=64 p99 1,147,317 us 1,126,141 us 1,056,513 us +1.9% +8.6%
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,495.36,200,128000,404,0.246,24406.93,30107.82,30107.82
1,100,10,64,20,2474.16,2000,1280000,808,0.493,123645.87,152475.66,152475.66
2,1000,10,64,20,21428.15,20000,12800000,933,0.570,1066640.37,1147317.28,1147317.28

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.28%. Comparing base (0ba5d1a) to head (287d1b1).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6822   +/-   ##
=========================================
  Coverage     76.28%   76.28%           
  Complexity     3452     3452           
=========================================
  Files          1161     1161           
  Lines         45916    45916           
  Branches       5099     5099           
=========================================
  Hits          35025    35025           
  Misses         9327     9327           
  Partials       1564     1564           
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 0ba5d1a
agent-service 76.76% <ø> (ø) Carriedforward from 0ba5d1a
amber 67.33% <ø> (ø) Carriedforward from 0ba5d1a
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from 0ba5d1a
config-service 66.66% <ø> (ø) Carriedforward from 0ba5d1a
file-service 67.21% <ø> (ø) Carriedforward from 0ba5d1a
frontend 81.72% <ø> (ø) Carriedforward from 0ba5d1a
notebook-migration-service 78.94% <ø> (ø) Carriedforward from 0ba5d1a
pyamber 92.15% <ø> (ø) Carriedforward from 0ba5d1a
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 0ba5d1a

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

@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

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 pybuilder BoundaryValidator and EncodableInspector

4 participants