You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "eugenegujing (via GitHub)" <gi...@apache.org> on 2026/06/15 04:18:11 UTC
[PR] test(workflow-core): add dedicated GlobalPortIdentitySerde spec [texera]
eugenegujing opened a new pull request, #5722:
URL: https://github.com/apache/texera/pull/5722
### What changes were proposed in this PR?
This PR adds a dedicated `GlobalPortIdentitySerdeSpec` for `GlobalPortIdentitySerde`, the helper that serializes/deserializes a `GlobalPortIdentity` to the compact, underscore-free string used in VFS URIs and file paths.
`GlobalPortIdentitySerde` was previously exercised inside `PortIdentitySerdeSpec`, which mixed it with the unrelated `PortIdentity` Jackson key (de)serializer tests. Rather than duplicate that coverage, this PR:
- **Moves** the `GlobalPortIdentitySerde` cases into their own file (the name issue #5717 asks for), so the two serde concerns are tested independently.
- **Trims** `PortIdentitySerdeSpec` down to only the `PortIdentity` Jackson key (de)serialization tests, dropping the now-unused imports.
- **Adds** seven edge cases that were previously unpinned (see below).
There are **no production-code changes** and **no duplication** between the two specs.
New edge cases added beyond the migrated coverage:
1. **serialize/deserialize asymmetry** — the deserializer regex accepts underscores even though the serializer rejects them; characterized so a future tightening of the regex breaks the test on purpose.
2. **unescaped comma** — a `logicalOpId` containing a `,` fails to round-trip, because the format does not escape its separator.
3. **portId boundary** — `Int.MaxValue` round-trips, and an out-of-Int-range value (`9999999999`) is rejected with `NumberFormatException` on deserialize.
4. **mixed-case booleans** — `True` / `FALSE` are accepted on deserialize, since Scala's `String.toBoolean` is case-insensitive.
5. **`=` in a value round-trips** — only `,` is a sensitive separator; an embedded `=` is captured fine, the counterpart to the comma case.
6. **empty `logicalOpId`** — serializes into `(logicalOpId=,...)` but the result can no longer be deserialized (`[^,]+` requires ≥1 char), the serialize side of the empty-field asymmetry.
7. **require messages name the field** — the serialize-side guards identify the offending field (`logicalOpId` / `layerName` / `portId`) in their message, catching a regression that wires a check to the wrong field.
| File | Change |
| --- | --- |
| `common/workflow-core/.../util/serde/GlobalPortIdentitySerdeSpec.scala` | **new** — 23 tests (`AnyFlatSpec with Matchers`) |
| `common/workflow-core/.../util/serde/PortIdentitySerdeSpec.scala` | removed the migrated `GlobalPortIdentitySerde` block + now-unused imports (−189 lines); `PortIdentity` Jackson key (de)serialization tests kept intact |
### Any related issues, documentation, discussions?
Closes #5717
### How was this PR tested?
Test-only PR. All checks below were run locally from the repository root and pass:
1. **Unit tests** — the new and modified specs:
```
sbt "WorkflowCore/testOnly org.apache.texera.amber.util.serde.GlobalPortIdentitySerdeSpec org.apache.texera.amber.util.serde.PortIdentitySerdeSpec"
```
Result: **31 tests, 0 failures** (1 `pendingUntilFixed`, pre-existing in `PortIdentitySerdeSpec`).
2. **Lint (scalafix)** — `sbt "scalafixAll --check"` → success.
3. **Format (scalafmt)** — `sbt scalafmtCheckAll` → success.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored by: Claude Opus 4.8
--
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(workflow-core): add dedicated GlobalPortIdentitySerde spec [texera]
Posted by "eugenegujing (via GitHub)" <gi...@apache.org>.
eugenegujing commented on PR #5722:
URL: https://github.com/apache/texera/pull/5722#issuecomment-4713083613
@Yicong-Huang Is there anything wrong with my PR here? Or do I need to change anything?
--
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(workflow-core): add dedicated GlobalPortIdentitySerde spec [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5722:
URL: https://github.com/apache/texera/pull/5722#issuecomment-4704527062
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5722?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 52.86%. Comparing base ([`15092f4`](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d128584`](https://app.codecov.io/gh/apache/texera/commit/d1285843a5261b9f54d99b17a729bf03f5ec8587?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 6 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #5722 +/- ##
=========================================
Coverage 52.86% 52.86%
Complexity 2622 2622
=========================================
Files 1090 1090
Lines 42184 42184
Branches 4530 4530
=========================================
Hits 22301 22301
Misses 18576 18576
Partials 1307 1307
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/5722/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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.91% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.36% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.97% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.65% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.71% <ø> (ø)` | | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.06% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.86% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.73% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [python](https://app.codecov.io/gh/apache/texera/pull/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.73% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.69% <ø> (ø)` | | Carriedforward from [15092f4](https://app.codecov.io/gh/apache/texera/commit/15092f4f986168725ae1a5b6db93d59d31dce006?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/5722?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(workflow-core): add dedicated GlobalPortIdentitySerde spec [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5722:
URL: https://github.com/apache/texera/pull/5722#issuecomment-4704541344
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 1 worse · ⚪ 12 noise (<±5%) · 0 without baseline
> Compared against main `83df2b5` 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/27523695206)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🟢 | bs=10 sw=10 sl=64 | 450 | 0.275 | 21,828/28,385/28,385 us | 🟢 -12.2% / 🟢 -18.9% |
| 🔴 | bs=100 sw=10 sl=64 | 933 | 0.569 | 106,843/129,770/129,770 us | 🔴 +6.3% / 🟢 -7.2% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,123 | 0.686 | 888,183/951,775/951,775 us | ⚪ within ±5% / 🟢 -8.7% |
<details><summary>Baseline details</summary>
Latest main `83df2b5` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 450 tuples/sec | 436 tuples/sec | 410.82 tuples/sec | +3.2% | +9.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.275 MB/s | 0.266 MB/s | 0.251 MB/s | +3.4% | +9.7% |
| bs=10 sw=10 sl=64 | p50 | 21,828 us | 22,005 us | 23,785 us | -0.8% | -8.2% |
| bs=10 sw=10 sl=64 | p95 | 28,385 us | 32,346 us | 34,980 us | -12.2% | -18.9% |
| bs=10 sw=10 sl=64 | p99 | 28,385 us | 32,346 us | 34,980 us | -12.2% | -18.9% |
| bs=100 sw=10 sl=64 | throughput | 933 tuples/sec | 967 tuples/sec | 891.94 tuples/sec | -3.5% | +4.6% |
| bs=100 sw=10 sl=64 | MB/s | 0.569 MB/s | 0.59 MB/s | 0.544 MB/s | -3.6% | +4.5% |
| bs=100 sw=10 sl=64 | p50 | 106,843 us | 100,469 us | 112,277 us | +6.3% | -4.8% |
| bs=100 sw=10 sl=64 | p95 | 129,770 us | 127,531 us | 139,802 us | +1.8% | -7.2% |
| bs=100 sw=10 sl=64 | p99 | 129,770 us | 127,531 us | 139,802 us | +1.8% | -7.2% |
| bs=1000 sw=10 sl=64 | throughput | 1,123 tuples/sec | 1,122 tuples/sec | 1,041 tuples/sec | +0.1% | +7.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.686 MB/s | 0.685 MB/s | 0.635 MB/s | +0.1% | +8.0% |
| bs=1000 sw=10 sl=64 | p50 | 888,183 us | 893,407 us | 972,714 us | -0.6% | -8.7% |
| bs=1000 sw=10 sl=64 | p95 | 951,775 us | 949,391 us | 1,023,057 us | +0.3% | -7.0% |
| bs=1000 sw=10 sl=64 | p99 | 951,775 us | 949,391 us | 1,023,057 us | +0.3% | -7.0% |
</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,443.98,200,128000,450,0.275,21828.47,28384.52,28384.52
1,100,10,64,20,2144.06,2000,1280000,933,0.569,106842.67,129769.55,129769.55
2,1000,10,64,20,17804.00,20000,12800000,1123,0.686,888183.15,951775.34,951775.34
```
</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(workflow-core): add dedicated GlobalPortIdentitySerde spec [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5722:
URL: https://github.com/apache/texera/pull/5722#issuecomment-4713270917
I think nothing related to your changes. CI is quite flaky, need to look into it.
--
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(workflow-core): add dedicated GlobalPortIdentitySerde spec [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang merged PR #5722:
URL: https://github.com/apache/texera/pull/5722
--
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