Skip to content

chore(ci): move LICENSE-binary version-drift tests into the infra job#5965

Open
Yicong-Huang wants to merge 2 commits into
apache:mainfrom
Yicong-Huang:chore/fold-license-into-infra
Open

chore(ci): move LICENSE-binary version-drift tests into the infra job#5965
Yicong-Huang wants to merge 2 commits into
apache:mainfrom
Yicong-Huang:chore/fold-license-into-infra

Conversation

@Yicong-Huang

@Yicong-Huang Yicong-Huang commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Move bin/licensing/test_*.py — the unit tests for the per-module LICENSE-binary version-drift checker — from pyamber into infra (the project-tooling CI lane added in #5961).

Before:                                  After:
─────────────────────────────────────    ─────────────────────────────────────
pyamber (ubuntu × 3.10/3.11/3.12/3.13)   infra (ubuntu, macos)
  ├─ Unit-test licensing scripts × 4       ├─ Unit-test licensing scripts × 1
  └─ pip-licenses / amber tests            ├─ Run shell smoke tests
                                           └─ Run Python unit tests (pytest
infra (ubuntu, macos)                          covers bin/licensing/ +
  ├─ Run shell smoke tests                     bin/local-dev/tests/)
  └─ Run Python unit tests

Same stdlib unittest.TestCase tests — pytest discovers them without rewriting. Single combined pytest bin/local-dev/tests/ bin/licensing/ invocation so test failures are easy to attribute.

The infra job grows from "dev-loop tooling only" to "any project-tooling under bin/* that isn't tied to a specific service". Comment + labeler glob updated to match; bin/licensing/** now picks up the job automatically.

Any related issues, documentation, discussions?

Closes #5964.

How was this PR tested?

Locally on this checkout:

$ python -m pytest bin/local-dev/tests/ bin/licensing/ -v --tb=short
============================== 91 passed in 0.27s ==============================

$ bash bin/local-dev/tests/test_local_dev_sh.sh
9 passed, 0 failed

53 licensing tests (test_check_binary_deps.py + test_generate_notice_binary.py) + 38 local-dev TUI / dirty-detection tests, all on Python 3.12.

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

Generated-by: Claude Code (Anthropic, Claude Opus 4.7).

`bin/licensing/test_check_binary_deps.py` (and friends) used to live as
a step inside the `pyamber` job, running the same stdlib unittests
four times — once per matrix row (3.10 / 3.11 / 3.12 / 3.13). They
have no pyamber dependency; 3.12 is the only row that actually drives
the binary-license check downstream. Consolidate them in the `infra`
job (project-tooling CI lane added in apache#5961) using pytest discovery
across both `bin/local-dev/tests/` and `bin/licensing/`.

Also broaden the `infra` job's scope from "dev-loop tooling only" to
"any project-tooling under bin/* that isn't tied to a specific
service". `bin/licensing/**` joins the `infra` labeler glob so changes
there pick up the job automatically; new tooling test suites should
land here too rather than spawning another CI job.

Closes apache#5964.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the ci changes related to CI label Jun 28, 2026
@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.

@codecov-commenter

codecov-commenter commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.24%. Comparing base (de6c31a) to head (1dd8a6c).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5965      +/-   ##
============================================
- Coverage     55.27%   55.24%   -0.03%     
+ Complexity     2990     2987       -3     
============================================
  Files          1117     1117              
  Lines         43258    43258              
  Branches       4668     4668              
============================================
- Hits          23910    23899      -11     
- Misses        17938    17947       +9     
- Partials       1410     1412       +2     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 34.36% <ø> (ø)
amber 57.77% <ø> (-0.02%) ⬇️
computing-unit-managing-service 0.00% <ø> (ø)
config-service 51.56% <ø> (ø)
file-service 59.02% <ø> (ø)
frontend 48.84% <ø> (-0.04%) ⬇️
notebook-migration-service 78.57% <ø> (ø)
pyamber 90.20% <ø> (ø)
python 90.76% <ø> (ø) Carriedforward from 73344eb
workflow-compiling-service 55.14% <ø> (ø)

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

Followup. The `infra` job is meant to host any project-tooling test
suite under `bin/`, but its two test steps were still hardcoded:

  bash bin/local-dev/tests/test_local_dev_sh.sh
  python -m pytest bin/local-dev/tests/ bin/licensing/ -v

Generalise both:

* Shell — `find bin/**/test_*.sh` and bash each one. New `bin/<svc>/
  tests/test_*.sh` files picked up without editing this workflow.
* Python — `pytest bin/` recurses through every `test_*.py` under
  bin/, same idea.

`set -e` on the bash loop so a failing test doesn't get masked by a
later passing one.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main de6c31a 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 408 0.249 23,431/33,994/33,994 us 🔴 +8.3% / 🔴 +119.8%
bs=100 sw=10 sl=64 799 0.488 122,479/147,114/147,114 us ⚪ within ±5% / 🔴 +33.7%
bs=1000 sw=10 sl=64 923 0.563 1,088,760/1,107,914/1,107,914 us ⚪ within ±5% / 🔴 +7.2%
Baseline details

Latest main de6c31a from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 408 tuples/sec 415 tuples/sec 756.6 tuples/sec -1.7% -46.1%
bs=10 sw=10 sl=64 MB/s 0.249 MB/s 0.253 MB/s 0.462 MB/s -1.6% -46.1%
bs=10 sw=10 sl=64 p50 23,431 us 21,642 us 13,009 us +8.3% +80.1%
bs=10 sw=10 sl=64 p95 33,994 us 35,100 us 15,463 us -3.2% +119.8%
bs=10 sw=10 sl=64 p99 33,994 us 35,100 us 18,561 us -3.2% +83.1%
bs=100 sw=10 sl=64 throughput 799 tuples/sec 829 tuples/sec 963.83 tuples/sec -3.6% -17.1%
bs=100 sw=10 sl=64 MB/s 0.488 MB/s 0.506 MB/s 0.588 MB/s -3.6% -17.0%
bs=100 sw=10 sl=64 p50 122,479 us 118,850 us 103,320 us +3.1% +18.5%
bs=100 sw=10 sl=64 p95 147,114 us 145,658 us 110,058 us +1.0% +33.7%
bs=100 sw=10 sl=64 p99 147,114 us 145,658 us 118,543 us +1.0% +24.1%
bs=1000 sw=10 sl=64 throughput 923 tuples/sec 928 tuples/sec 989.07 tuples/sec -0.5% -6.7%
bs=1000 sw=10 sl=64 MB/s 0.563 MB/s 0.567 MB/s 0.604 MB/s -0.7% -6.7%
bs=1000 sw=10 sl=64 p50 1,088,760 us 1,083,236 us 1,015,599 us +0.5% +7.2%
bs=1000 sw=10 sl=64 p95 1,107,914 us 1,160,830 us 1,055,944 us -4.6% +4.9%
bs=1000 sw=10 sl=64 p99 1,107,914 us 1,160,830 us 1,086,834 us -4.6% +1.9%
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,490.52,200,128000,408,0.249,23431.22,33993.81,33993.81
1,100,10,64,20,2502.69,2000,1280000,799,0.488,122479.19,147113.70,147113.70
2,1000,10,64,20,21674.22,20000,12800000,923,0.563,1088759.57,1107914.24,1107914.24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci changes related to CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move LICENSE-binary version-drift script tests into the infra CI job

2 participants