Skip to content

chore(pybuilder): aggregate PyBuilder at root and add API spec for non-macro pieces#5024

Merged
Yicong-Huang merged 5 commits into
apache:mainfrom
Yicong-Huang:pybuilder-aggregate-and-tests
May 11, 2026
Merged

chore(pybuilder): aggregate PyBuilder at root and add API spec for non-macro pieces#5024
Yicong-Huang merged 5 commits into
apache:mainfrom
Yicong-Huang:pybuilder-aggregate-and-tests

Conversation

@Yicong-Huang
Copy link
Copy Markdown
Contributor

@Yicong-Huang Yicong-Huang commented May 11, 2026

What changes were proposed in this PR?

Three closely related changes scoped to common/pybuilder:

  1. Add PyBuilder to the root TexeraProject aggregate(...), grouped under "common libraries" alongside Auth, Config, DAO, WorkflowCore, and WorkflowOperator (alphabetized within each group). Before this PR, PyBuilder was the only sbt project defined but not aggregated, so sbt test and sbt scalafmtCheckAll from the repo root silently skipped it. CI still ran PyBuilder/jacoco explicitly, so coverage reporting was unaffected, but the local/CI matrix mismatch was confusing.

  2. Apply scalafmt to four pre-existing PyBuilder files (BoundaryValidator.scala, EncodableInspector.scala, PythonTemplateBuilder.scala, PythonTemplateBuilderSpec.scala). scalafmtCheckAll only iterates over aggregated sub-projects, so change 1 brought PyBuilder under the format gate for the first time and surfaced debt that had been accumulating invisibly since the project was introduced. Reformatting these files is the necessary follow-on for change 1 to leave CI green; the diffs are purely scaladoc-star indentation and multi-line case-class parameter alignment, no semantic changes.

  3. Add PythonTemplateBuilderApiSpec covering non-macro pieces of PythonTemplateBuilder that PythonTemplateBuilderSpec only touches incidentally — factory constructors, render-mode singletons, renderer behavior in both modes, fromInterpolated precondition, +(String) UnsupportedOperationException, and render() CR/CRLF normalization. Also pins current PythonLexerUtils behavior on Python triple-quoted strings (the lexer is intentionally conservative and not triple-quote-aware; pinned so a future triple-quote-aware change trips the spec deliberately).

Any related issues, documentation, discussions?

Resolves #5023

How was this PR tested?

sbt PyBuilder/test — 125 tests pass (32 new). sbt PyBuilder/jacoco line coverage went from 30.12% to 31.73% and method coverage from 13.17% to 15.30% (6 newly covered methods); the rest of PythonTemplateBuilder.scala is the pybImpl macro body which jacoco cannot instrument at runtime. sbt scalafmtCheckAll now passes cleanly with PyBuilder included.

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

Generated-by: Claude Opus 4.7 (1M context)

…n-macro pieces

PyBuilder was defined as an sbt project but missing from the root
TexeraProject aggregate(...), so `sbt test` from the repo root silently
skipped it (CI invokes `PyBuilder/jacoco` explicitly so coverage is
still reported). Group the aggregate list into "common libraries" and
"services" while adding PyBuilder under the common section.

Add PythonTemplateBuilderApiSpec covering the non-macro surface that
PythonTemplateBuilderSpec only exercises incidentally: factory
constructors, RenderMode singletons, EncodableStringRenderer /
PyLiteralStringRenderer mode behavior, fromInterpolated parts/args
precondition, +(String) UnsupportedOperationException, CR/CRLF
normalization, and pinned PythonLexerUtils behavior on Python
triple-quoted strings (the lexer is intentionally conservative; pinned
so a future triple-quote-aware change breaks the spec deliberately).

Refs apache#5023

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file common labels May 11, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 11, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.70%. Comparing base (0e9c617) to head (bba989b).

Files with missing lines Patch % Lines
...che/texera/amber/pybuilder/BoundaryValidator.scala 0.00% 9 Missing ⚠️
...he/texera/amber/pybuilder/EncodableInspector.scala 0.00% 4 Missing ⚠️
...texera/amber/pybuilder/PythonTemplateBuilder.scala 57.14% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5024      +/-   ##
============================================
+ Coverage     42.48%   42.70%   +0.22%     
+ Complexity     2185     2184       -1     
============================================
  Files          1045     1031      -14     
  Lines         39878    38094    -1784     
  Branches       4205     4004     -201     
============================================
- Hits          16941    16268     -673     
+ Misses        21875    20807    -1068     
+ Partials       1062     1019      -43     
Flag Coverage Δ *Carryforward flag
access-control-service 39.88% <ø> (ø)
agent-service 33.72% <ø> (ø) Carriedforward from 16d3c6a
amber 43.24% <20.00%> (+<0.01%) ⬆️
computing-unit-managing-service 0.00% <ø> (ø)
config-service 0.00% <ø> (ø)
file-service 32.10% <ø> (ø)
frontend 32.99% <ø> (-0.52%) ⬇️ Carriedforward from 16d3c6a
python 88.86% <ø> (-0.04%) ⬇️ Carriedforward from 16d3c6a
workflow-compiling-service 47.72% <ø> (ø)

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

scalafmtCheckAll only iterates over sub-projects in the root aggregate.
PyBuilder was excluded from that iteration before this branch added it,
so these 4 files had been unformatted on main without CI flagging them.
Running PyBuilder/scalafmt makes scalafmtCheckAll pass.

No semantic changes — scaladoc star indentation and case class
multi-line parameter alignment only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns local sbt test behavior with CI by aggregating the PyBuilder subproject under the root TexeraProject, and expands unit coverage for the non-macro API surface of PythonTemplateBuilder (factories, render modes, render normalization, concatenation behavior, and preconditions).

Changes:

  • Add PyBuilder to the root SBT aggregate list so root-level sbt test includes PyBuilder tests.
  • Add PythonTemplateBuilderApiSpec to directly test non-macro API behaviors and pin current lexer behavior around Python triple quotes.
  • Apply formatting-only refactors in existing Scala sources/specs (no intended behavioral changes).

Reviewed changes

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

Show a summary per file
File Description
build.sbt Adds PyBuilder to TexeraProject.aggregate(...) to ensure root builds/tests include it.
common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/PythonTemplateBuilderApiSpec.scala New spec covering non-macro PythonTemplateBuilder API and pinning lexer behavior.
common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/PythonTemplateBuilderSpec.scala Formatting adjustments only.
common/pybuilder/src/main/scala/org/apache/texera/amber/pybuilder/PythonTemplateBuilder.scala Formatting/minor refactor of chaining; no functional changes intended.
common/pybuilder/src/main/scala/org/apache/texera/amber/pybuilder/EncodableInspector.scala Formatting-only adjustments.
common/pybuilder/src/main/scala/org/apache/texera/amber/pybuilder/BoundaryValidator.scala Formatting-only adjustments.

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

Yicong-Huang and others added 2 commits May 10, 2026 18:48
Keep the two-group split readable, but sort each group A-Z so future
additions have an obvious slot.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This spec exercises the non-macro API directly via fromInterpolated;
the pyb"..." interpolator is not used here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@bobbai00 bobbai00 left a comment

Choose a reason for hiding this comment

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

I see lots of formatting change. Are they needed ? If not can you revert them ?

Other parts LGTM

@Yicong-Huang
Copy link
Copy Markdown
Contributor Author

I see lots of formatting change. Are they needed ? If not can you revert them ?

Other parts LGTM

I explained in the PR description already ;)

@Yicong-Huang Yicong-Huang enabled auto-merge (squash) May 11, 2026 06:24
@Yicong-Huang Yicong-Huang merged commit 2a23ba5 into apache:main May 11, 2026
17 checks passed
@Yicong-Huang Yicong-Huang deleted the pybuilder-aggregate-and-tests branch May 11, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add PyBuilder to root aggregate and cover non-macro gaps with unit tests

4 participants