You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/07/23 06:32:20 UTC
[PR] test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector [texera]
aglinxinyuan opened a new pull request, #6822:
URL: https://github.com/apache/texera/pull/6822
### 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])
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6822:
URL: https://github.com/apache/texera/pull/6822#issuecomment-5055219059
<!-- texera-reviewer-suggestion -->
### 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6822:
URL: https://github.com/apache/texera/pull/6822#issuecomment-5055290052
<!-- texera-benchmarks-comment -->
## ⚠️ 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](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/29985560625)
| | 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% |
<details><summary>Baseline details</summary>
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% |
</details>
<details><summary>Raw CSV</summary>
```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
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6822:
URL: https://github.com/apache/texera/pull/6822#discussion_r3636038297
##########
common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/EncodableInspectorSpec.scala:
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.amber.pybuilder
+
+import org.scalatest.funsuite.AnyFunSuite
+
+import scala.reflect.runtime.currentMirror
+import scala.tools.reflect.{ToolBox, ToolBoxError}
+
+/**
+ * Runtime coverage for `EncodableInspector`.
+ *
+ * `EncodableInspector` only executes while the `pyb"..."` macro expands. In an ordinary build
+ * that expansion happens before Jacoco is attached, so none of its classification logic is
+ * observed at runtime. To exercise it inside the instrumented JVM we drive real macro expansions
+ * through a runtime `ToolBox`.
+ *
+ * We cannot `eval` a `pyb` snippet: the macro expands to a call to the `private[amber]`
+ * `PythonTemplateBuilder.fromInterpolated`, which the ToolBox's synthetic `__wrapper` package
+ * cannot access, so evaluation always fails on that access check. Compilation is still useful,
+ * though, because the macro fully expands (running the whole inspector) *before* that
+ * post-typecheck access error surfaces. That gives two observable outcomes we assert on:
+ *
+ * - '''boundary abort''': the inspector classified a *direct* argument as Encodable, so
+ * `BoundaryValidator.validateCompileTime` ran and aborted with a `@EncodableStringAnnotation`
+ * message (only possible when the arg was deemed Encodable and the context is unsafe);
+ * - '''benign expansion''': no boundary abort fired and the only failure is the private-access
+ * error on `fromInterpolated`, i.e. the inspector deemed the arg a plain literal (or a nested
+ * builder), the macro finished expanding, and lowering/`wrapArg` ran.
+ *
+ * Placing an Encodable-marked value next to a "bad neighbor" therefore proves it was classified
+ * Encodable; placing a plain value there proves it was not.
+ */
+class EncodableInspectorSpec extends AnyFunSuite {
+
+ private lazy val tb: ToolBox[scala.reflect.runtime.universe.type] = currentMirror.mkToolBox()
+
+ private val header =
+ """import org.apache.texera.amber.pybuilder.PythonTemplateBuilder._
+ |import org.apache.texera.amber.pybuilder.PyStringTypes._
+ |import org.apache.texera.amber.pybuilder.EncodableStringAnnotation""".stripMargin
+
+ /** Marker present in every `BoundaryValidator` compile-time abort message. */
+ private val boundaryMarker = "@EncodableStringAnnotation argument #"
+
+ /** Compile a self-contained `pyb` snippet; it always fails, so capture the ToolBox message. */
+ private def macroError(body: String): String =
+ intercept[ToolBoxError] {
+ tb.compile(tb.parse(s"{\n$header\n$body\n}"))
+ }.getMessage
+
+ /** Assert the arg was classified Encodable: an unsafe splice triggers a boundary abort. */
+ private def assertClassifiedEncodable(body: String): Unit = {
+ val msg = macroError(body)
+ assert(msg.contains(boundaryMarker), s"expected an Encodable boundary abort, got: $msg")
+ }
+
+ /** Assert the arg was NOT classified Encodable: the macro expands, only fromInterpolated fails. */
+ private def assertNotClassifiedEncodable(body: String): Unit = {
Review Comment:
The Scaladoc for `assertNotClassifiedEncodable` is inaccurate: it says the arg was “NOT classified Encodable”, but the helper is also used by the “safe Encodable splice…” test (where the arg *is* Encodable, just not aborting). This can mislead future readers about what the assertion actually guarantees.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6822:
URL: https://github.com/apache/texera/pull/6822#issuecomment-5055480573
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/6822?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 76.28%. Comparing base ([`0ba5d1a`](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`287d1b1`](https://app.codecov.io/gh/apache/texera/commit/287d1b1f39955fcf4e9a1b812e08b0954da33b63?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:white_check_mark: All tests successful. No failed tests found.
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.76% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.33% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `20.49% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.66% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.21% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.72% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.94% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `92.15% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/6822/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.14% <ø> (ø)` | | Carriedforward from [0ba5d1a](https://app.codecov.io/gh/apache/texera/commit/0ba5d1a22a3baaa61a36d1bbb8c4573d40e89b5c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/6822?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #6822:
URL: https://github.com/apache/texera/pull/6822
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org