You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Cross-attempt artifact reuse and workflow gating depend on GitHub Actions behavior that cannot be fully validated statically.
Review effort: Balanced Findings: None
Hangyi (HangyiWang)
changed the title
perf(ci): parallelize GitHub unit prechecks with reusable shards
perf(ci): parallelize unit tests (~20 min to ~6 min)
Oct 6, 2026
The sharding machinery itself is carefully fail-closed and I could not find a hole in it — validate rejects missing/duplicate shards, stale attempts, changed artifacts, non-terminal sessions and per-stage report anomalies, and the gate's needs: [lint, unit-shards] with no if: always() correctly keeps int-test from running when a shard fails. The findings below are about scope and about one new test's portability.
Blocking
None.
Suggestions
The speedup does not apply to pull-request CI..github/workflows/int_test.yml triggers only on workflow_call and workflow_dispatch (~lines 6 and 46) — it has no pull_request trigger, and its only callers are release_workflow.yml and int_test_schedule.yml. PR unit testing comes from ci_workflow.yml (on: pull_request, push) → tox.yml, which is untouched here and still runs the 12-cell os × py matrix with the full tox r (clean, lint, python-azcur-unit, report) in every cell. So the "20m → 6m" in the title is a release/scheduled-run improvement only. Either say so in the title/description, or port the same shard job into tox.yml where the serial 20 minutes is actually paid 12 times.
azext_iot/tests/test_unit_shards_unit.py ~line 635 — test_real_four_process_collection_and_coverage_with_random_parameters has no platform guard. It spawns four concurrent subprocess.run([sys.executable, "-m", "pytest", ...], timeout=90) through a ThreadPoolExecutor, and because it is a *_unit.py file it will be collected by tox.yml on windows-2025 and macos-15-intel as well. Every other heavy subprocess/real-process test in this repo carries an explicit guard — test_dps_phase_lifetime_unit.py (~304, ~386), test_dps_phase_runtime_unit.py (~784, ~1150), test_dps_phase_runner_unit.py (~809, ~856, ~1025, ~1039), test_hub_ownership_transport_unit.py (~35, module-level pytestmark), test_workflow_results_unit.py (~303, ~487). A hard 90 s budget for four parallel full pytest sessions on the slower Windows/macOS runners is a plausible flake; a @pytest.mark.skipif(sys.platform != "linux", ...) would match the established convention. (test_github_prechecks_... and test_tox_passes_... are file reads and are fine everywhere.)
azext_iot/tests/unit_test_durations.json omits the PR's own new test file.partition() falls back to default_seconds: 1 for unknown files, so azext_iot/tests/test_unit_shards_unit.py — which by inspection is one of the more expensive files in the suite (four real pytest subprocesses plus a tmp_path coverage combine) — will be treated as the cheapest file and appended last to whichever shard is lightest. Add a measured entry in the same commit, otherwise the very first run of the new balancer is skewed by the balancer's own tests.
The gate never checks the Python series against the requested one._unit_shards.validate asserts len({python_series(record["python"]) for record in records}) == 1 — agreement, not identity. If actions/setup-python resolved 3.12 on all four runners the gate would still pass, despite test_unit_gate_accepts_different_patch_releases_within_the_requested_python_series and the CONTRIBUTING wording both framing this as "the requested series". Passing the expected series through (e.g. a UNIT_PYTHON_SERIES: "3.13" env compared alongside context()) makes the assertion match its name.
clean and report silently drop out of this workflow. The old step was tox r --skip-pkg-install, which runs the whole envlist (clean, lint, python-azcur-unit, report); the shard job now runs -e python-azcur-unit only. That is correct for clean (each shard has its own COVERAGE_FILE), but it also means coverage report/html/json no longer execute in this workflow. Harmless today since report has no --fail-under, but worth a deliberate note if a coverage threshold is ever added there.
Nits
.github/workflows/int_test.yml — max-parallel: 4 on a four-entry matrix is a no-op; it only exists because test_github_prechecks_keep_lint_independent_and_gate_every_shard asserts the exact strategy dict (and test_workflow_parallelism_unit.py then has to special-case unit-shards out of its max-parallel ban). Dropping it from both would remove the exemption.
azext_iot/tests/_unit_shard_plugin.py ~line 214 — shards.write(self.output / "receipt.json", self.data, exclusive=True) raises a bare FileExistsError when a developer reuses a --unit-shard-output directory locally. Every other misuse in pytest_configure raises pytest.UsageError with an explanation; this one is the likeliest to be hit by hand.
--junitxml is now supplied twice — tox.ini has --junitxml=junit/test-iotext-unit.xml and the workflow appends --junitxml "$RESULTS/junit.xml" via posargs. Last wins, so this is intentional, but it means the tox-local junit file is no longer produced in CI and the duplication is easy to misread.
Checked and clean
Lint is fine: setup.cfg sets max-line-length = 130, so the 126- and 122-character lines in _unit_shards.py and test_unit_shards_unit.py are not E501 failures (confirmed by running flake8 at this head).
python-azcur-unit is a real environment — tox.inienvlist names it, and the python factor means "local interpreter", i.e. the 3.13 installed by setup-python.
The plugin's strictest guard holds: pytest_configure requires config.args == [rootpath / "azext_iot/tests"], and tox's changedir defaults to toxinidir, so the tox command's ./azext_iot/tests resolves to exactly that. The same guard is satisfied in the subprocess test via -c tmp_path/pytest.ini.
Artifact digests cannot be empty by accident: Receipt.pytest_sessionfinish is a tryfirsthookwrapper, so its post-yield body runs after pytest-cov's and LogXML's pytest_sessionfinish have written coverage.dat and junit.xml. Relatedly, Receipt.__init__ is what creates $RESULTS at configure time, which is what makes COVERAGE_FILE=unit-result/coverage.dat writable at save time.
Rerun semantics work: github.run_id is stable across attempts (so the context equality holds), download-artifact with pattern: unit-shard-* sees every attempt in the run, and aggregate takes the max attempt per shard — matching the empty-newer-attempt / wrong-attempt cases in the new tests.
Downstream coverage still resolves: combine-coverage globs coverage-* artifacts for files literally named .coverage, the gate uploads unit-coverage/.coverage as coverage-unit, and the shard artifacts carry coverage.dat, so there is no double counting.
azext_iot/tests/digitaltwins/test_dt_resource_unit.py is a strict improvement: Retry-After: "0" plus result.result(timeout=10) removes two sleep(10) spin loops, LROPoller.wait() re-raises so pytest.raises(CloudError) around result(timeout=10) is correct, and the added assert result.done() covers the case where the timeout elapses and result() would otherwise return None silently.
CI/test-only change, so no HISTORY.rst section or azext_iot/constants.pyVERSION bump is expected.
This is an automated review and may be incomplete.
Add measured duration for test_unit_shards_unit.py
azext_iot/tests/unit_test_durations.json:3
The profile omits the newly added azext_iot/tests/test_unit_shards_unit.py, so it receives this one-second default even though that file launches four nested pytest processes and combines coverage. This systematically underweights one of the new suite's expensive files and weakens the balancing this PR is intended to provide. Add its measured duration from a complete run to seconds.
Two commits: 6f875adc ports the four-shard runner into .github/workflows/tox.yml (PR CI) with a per-combination unit-gate, and 319406b7 clarifies the artifact naming in CONTRIBUTING.md. This closes suggestion 1 of my previous review — the shards now apply to pull-request CI, and the measured result is good (numbers below). Suggestion 5 (report dropping out) is also closed, because the gate runs coverage report/html/json explicitly.
Blocking
None.
Suggestions
The PR critical path is now the Azure DevOps Merge pipeline, not this workflow, so the user-visible wall clock is unchanged..azure-devops/merge.yml has a pr: trigger (line 11) and runs the unsharded unit suite in run_unit_tests_ubuntu (3.10/3.11/3.12/3.13, line 51), run_unit_tests_macOs (3.12, line 79) and run_unit_tests_windows (3.12, line 95). Measured on this head's check runs: all 48 GitHub shards plus the 12 gates completed by 19:49:13Z, while Merge … (run_unit_tests_windows) took 28.4 min and run_unit_tests_macOs was still in_progress at 20:01Z (>28 min). So a contributor waiting on a green PR still waits ~30 min. Worth either sharding those ADO jobs through the same _unit_shards.py runner (the shared runner already "accepts CI-neutral run, commit, and attempt inputs so Azure Pipelines can use the same implementation", per CONTRIBUTING) or retiring them now that GitHub covers 12 combinations instead of ADO's 6.
Run lint once per OS and Python is bolted onto shard 1, which defeats the balanced partition..github/workflows/tox.yml gates it on if: ${{ matrix.shard == 1 }}inside the tox job, so shard 1 is systematically the longest job in every combination and the unit-gate for that combination waits on it. Step timings from run 37519314610, ubuntu-24.04 / 3.13:
shard
Setup
Run test suite
Lint
1
1.07m
4.28m
3.20m
2
1.15m
4.38m
skipped
3
1.10m
4.57m
skipped
4
0.97m
4.85m
skipped
Across all 48 shard check runs the pattern holds: shard 1 mean 12.4m / max 18.7m, versus 7.5–8.8m mean and 10.5–13.5m max for shards 2–4. int_test.yml already models the right shape — lint is an independent job there ("needs" not in lint in test_github_prechecks_keep_lint_independent_and_gate_every_shard). A separate 12-cell lint job would take ~3–5 min off the critical path of every combination. Note also that Setup test suite only pre-creates python-azcur-unit (-e python-azcur-unit … --notest), so the lint step pays its own env creation inside the timed step.
The job rename changes every PR check-run name — confirm branch protection before merging. Unlike int_test.yml, tox.ymldoes produce PR check runs (via ci_workflow.yml, on: pull_request). At 03f6782d the 12 names were test / Unit test <py> - <os>; at this head they are test / Unit test <py> - <os> (shard N/4) plus 12 new test / Unit gate <py> - <os>. If any of the old names is a required status check on dev / preview / release/*, those contexts stop reporting and PRs block indefinitely. I cannot read the protection settings (the API returns 404 for this token), so this is a request to verify rather than a confirmed break — and if the gate is meant to be the required contract, Unit gate … is the better name to require, since unit-gate has no if: always() and is skipped when any shard fails.
azext_iot/tests/unit_test_durations.json still has no entry for test_unit_shards_unit.py, and this increment made that file heavier. Suggestion 3 from the previous review is still open, and test_real_four_process_collection_and_coverage_with_random_parameters plus test_latest_successful_native_unit_attempt_is_aggregated are now @pytest.mark.parametrize("prefix", [...]) with two values, so the real-subprocess test runs twice. partition() falls back to default_seconds for unknown files and appends them to the lightest shard, so the balancer's own most expensive file is still weighted as the cheapest — on 12 combinations rather than 1.
Suggestion 4 (the gate asserts the four receipts agree on a Python series, not that they match the requested series) is unchanged by this increment and now matters 12× rather than once, since each gate is per-matrix.py. Copilot raised the same point inline on _unit_shards.py (4199170049); I am not restating the detail, only noting it survives at this head.
Nits
The platform-guard point from my last review (suggestion 2) is now empirically weaker — every one of the 48 shard check runs on this head is green, including all windows-2025 and macos-15-intel cells, so the 90 s budget for four concurrent pytest sessions holds in practice on the slower runners. Doubling the test via the prefix parametrize eats into that margin though, and a skipif(sys.platform != "linux") would still match the convention of every other heavy subprocess test in the repo. Downgrading it from a suggestion to a nit on the strength of the CI evidence.
azext_iot/tests/_unit_shards.py ~line 97 — aggregate now takes a prefix, but the mismatch error is still the literal "Unexpected unit-shard artifact.". With --prefix tox-unit-macos-15-intel-py3.12 that message names the wrong artifact family; interpolating prefix costs nothing.
Checked and clean
The speedup is real and nearly free. Comparing check-run timings at 03f6782d (12 jobs, max 38.3m, wall 38.4m, 622 runner-min) against this head (48 shards + 12 gates, max shard 18.7m, wall ~19m + ~1m gate, 555 + 14 = 569 runner-min): wall clock for this workflow halves and total runner-minutes go down. The 5× job-count increase does not cost more compute, because the ~1m setup is small relative to the serial test time it replaces.
Artifact namespaces cannot collide. The tox job uploads tox-unit-<os>-py<py>-<shard>-<attempt>, unit-gate downloads only pattern: tox-unit-<os>-py<py>-*, and aggregate's glob/regex are both anchored on the escaped prefix. No py<x> value is a prefix of another (3.10–3.13 are all 4 characters), so no cross-combination bleed. test_latest_successful_native_unit_attempt_is_aggregated now seeds a second prefix into the same directory to prove the isolation.
The gate is still fail-closed for real failures, not just for missing artifacts: validate rejects record["exitstatus"] != 0, finished is not True, and any per-case stage set other than passed/skipped (_unit_shards.py ~lines 70–85). And continue-on-error defaults to false in tox.yml's workflow_call inputs, with ci_workflow.yml calling it with no with: block, so the PR path never runs in tolerate-failure mode.
unit-gate having needs: tox with no if: is correct — a failed shard skips the gate rather than letting it pass on partial evidence, which is the same contract as unit-test in int_test.yml.
The clean / report envs dropping out of tox r is now deliberately replaced: each shard gets its own COVERAGE_FILE (so clean is unnecessary) and Generate coverage reports runs coverage report && coverage html && coverage json against the combined unit-coverage/.coverage, with htmlcov/ still uploaded only from ubuntu-24.04 / 3.13.
CONTRIBUTING.md's arithmetic checks out: 3 OS × 4 Python × 4 shards = 48 unit jobs, 12 gates, lint once per combination.
.read_text(encoding="utf-8") added to the two workflow-parsing tests is a genuine Windows fix, not cosmetic — those tests now run on windows-2025 under this very change, where the default encoding is not UTF-8.
CI/test-only change, so no HISTORY.rst section or azext_iot/constants.pyVERSION bump is expected.
This is an automated review and may be incomplete.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The timing profile underweights the newly added process-heavy shard tests, weakening the intended balancing.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Add measured duration for test_unit_shards_unit.py
azext_iot/tests/unit_test_durations.json:3
The newly added test_unit_shards_unit.py is absent from seconds, so its two four-process pytest/coverage cases receive this 1-second fallback even though they are substantially heavier than the files this default is meant to cover. That systematically underweights the shard runner's own tests and weakens the duration balancing; please add a measured duration from a complete run.
Extract reusable duration-balanced serial unit partitions and strict completeness/coverage aggregation from the ADO migration. Run four GitHub integration precheck shards alongside independent lint, with attempt-scoped evidence and CI-neutral run identity. Preserve the existing tox platform matrix and remove mocked Digital Twins polling sleeps.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hosted runners resolved Python 3.13 to both 3.13.15 and 3.13.16 in the same workflow. Validate the requested major/minor series while retaining exact versions in receipts and summaries; preserve all collection, execution and artifact checks. Add mixed-patch and incompatible-version regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve all OS/Python combinations with four serial pytest shards each, lint once per combination, and per-combination coverage gates. Namespace artifacts and run identities to isolate PR matrix results from integration prechecks. Reuse the existing aggregator with an optional prefix and fix Windows UTF-8 workflow reads.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the legacy required contexts on complete aggregate results and explicitly fail gates when shards or lint fail instead of skipping them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The branch was rebased onto an updated release/1.1.0-preview since the last review, so GET /compare is polluted with base-branch churn (the MQTT 5 / topic-group work from #907, #909, #915, #916). I diffed the PR's own 11 files blob-by-blob between 319406b7 and 51ef7cc8 instead. The real increment is small and touches three files: .github/workflows/tox.yml, CONTRIBUTING.md, and azext_iot/tests/test_unit_shards_unit.py.
This is a correct fix for a genuine hole, and I could not find a defect in it.
Blocking
None.
Suggestions
.github/workflows/tox.yml (unit-gate, ~line 77) — if: ${{ always() }} also makes the gate run when the workflow is cancelled, and the guard step then fires on needs.tox.result == 'cancelled' and exits 1. The practical effect is that a cancelled run surfaces 12 red Unit test <py> - <os> checks rather than cancelled ones. If that noise matters, if: ${{ !cancelled() }} keeps the whole point of the change (it still runs on failure/skipped) while leaving cancellation alone. Not a correctness issue — just reporting hygiene.
Nits
azext_iot/tests/test_unit_shards_unit.py (~line 255) — the new assertion assert "continue-on-error" not in guard locks the step, which is right, but the gate job still carries continue-on-error: ${{ inputs.continue-on-error }}. So the guard only hard-fails while that input is false. It is false for the PR path (the input defaults to false and ci_workflow.yml's test: job passes no with:, both of which the new assertions pin), so nothing is wrong today; it is worth a one-line comment that a workflow_dispatch with continue-on-error: true intentionally keeps the gate green.
Checked and clean
The rename genuinely restores the required check names. On the merge target release/1.1.0-preview, tox.yml's single job is tox named Unit test ${{ matrix.py }} - ${{ matrix.os }}, so the pre-PR required contexts are test / Unit test <py> - <os>. After this commit the gate carries exactly that name and the sharded job becomes ... (shard ${{ matrix.shard }}/4) — distinct strings, no collision, and the previously required contexts keep existing.
The hole being closed is real, not theoretical. Before this commit the gate was needs: tox with no if, so a failing shard skipped the gate, and GitHub reports a skipped job as passing for required status checks — i.e. the required Unit test <py> - <os> check would have gone green on a red shard. if: always() plus an explicit first-step guard is the standard fix and is the minimal one.
The guard covers lint too, so CONTRIBUTING.md's new sentence ("fail if any shard or lint run fails") is accurate: lint is a step inside the tox job (Run lint once per OS and Python, gated on matrix.shard == 1), so a lint failure fails that job and lands in needs.tox.result.
The guard is the gate's first step, before setup-python/checkout/download-artifact, so a failed shard set cannot reach the coverage-combine path with partial receipts.
Cross-platform safe: the run: block is echo "::error::..." + exit 1, which behaves identically under bash (ubuntu/macOS) and pwsh (windows-2025) — no $(...) subexpression, and exit 1 fails the step in both shells.
The new assertions match the file they assert against, character for character: gate["name"] == "Unit test ${{ matrix.py }} - ${{ matrix.os }}", unit["name"] == gate["name"] + " (shard ${{ matrix.shard }}/4)", gate["if"] == "${{ always() }}", and guard["if"] == "${{ needs.tox.result != 'success' }}". The previous revision's "if" not in gate assertion was correctly inverted rather than deleted.
The new caller assertions on .github/workflows/ci_workflow.yml hold: test: is uses: ./.github/workflows/tox.yml with no name: (so the check prefix really is test / ) and no with: block, which is why .get("continue-on-error", False) is False passes.
docs/tox-testing.md, .github/workflows/int_test.yml, tox.ini, _unit_shards.py, _unit_shard_plugin.py, unit_test_durations.json and test_workflow_parallelism_unit.py are unchanged in this increment — all of their deltas in the compare view come from the rebase, not from new work, so nothing from the previous two reviews needs re-deriving.
CI-only change, so no HISTORY.rst entry or azext_iot/constants.pyVERSION bump is expected.
Not restating Copilot's open inline comment on azext_iot/tests/_unit_shards.py (receipts agreeing with each other vs. agreeing with the matrix-requested Python series) — it predates this increment and is untouched by it.
This is an automated review and may be incomplete.
Keep pull-request coverage for every target while retaining push CI for long-lived branches and tags, plus manual dispatch.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep Python 3.13 on all three operating systems and Python 3.10 on Ubuntu for PRs. Default reusable, push, manual, and release runs to the full matrix, preserving required gates and complete per-combination test coverage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the four GitHub PR unit combinations and preserve every non-unit Merge job, including manifest generation and CredScan. Leave full GitHub matrices and integration pipelines unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The duration profile underweights the new process-heavy shard test, weakening the intended balancing.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Add measured duration for test_unit_shards_unit.py
azext_iot/tests/unit_test_durations.json:3
test_unit_shards_unit.py is absent from seconds, so this 1-second fallback is used even though that new file runs two four-process pytest/coverage cases. This systematically underweights one of the heavier new files and weakens the duration balancing this PR introduces; add its measured duration from a complete run.
Three PR-own commits: cce797ca (narrow the push: trigger), 4612e60a (reduce the PR unit matrix to four combinations), 67b1ecba (delete the ADO Merge unit jobs). The azext_iot/_help.py / azext_iot/adr/_help.py deltas in the compare view come from d9f5ce02 ("docs: format ARM endpoint help as literals (#919)") arriving via the base-branch merge, not from new work here — nothing to review in them.
Blocking
.github/workflows/tox.yml (~line 100) + .github/workflows/ci_workflow.yml (~line 25) — the reduced PR matrix removes 8 of the 12 required unit check names from pull_request runs, and a never-created required check does not go green.
The gate job is name: Unit test ${{ matrix.py }} - ${{ matrix.os }} under caller job test:, so the contexts are test / Unit test <py> - <os>. A combination removed by exclude: produces no job and no check run at all — GitHub reports a required-but-absent context as "Expected — Waiting for status to be reported", which blocks the merge button indefinitely rather than passing. Surviving on PRs: 3.13 × {ubuntu-24.04, windows-2025, macos-15-intel} and 3.10 × ubuntu-24.04. Disappearing on PRs: 3.12 × 3 OS, 3.11 × 3 OS, 3.10 × {windows-2025, macos-15-intel}.
This is not hypothetical on this branch: commit 51ef7cc8 in this same PR is literally "fix(ci): preserve required unit check names on shard gates", so the name contract is known to be load-bearing here. And CONTRIBUTING.md (~line 121) still asserts the old guarantee verbatim — "PR aggregate gates retain the test / Unit test <python> - <os> names required by branch rules" — which after this commit is true for 4 of the 12 names on PRs, so the doc and the workflow now disagree.
I cannot read the ruleset to confirm which contexts are required (GET /repos/.../branches/release%2F1.1.0-preview/protection returns 404 for this token), so the concrete ask is: before merging, check the required-status-check list on dev, preview and release/**, and if any of those 8 names is required, drop them from the ruleset in the same change. Note that narrowing exclude: to only the tox job is not a workaround — the surviving gate jobs would find no matching tox-unit-<os>-py<py>-* artifacts and fail in _unit_shards.py.
Suggestions
.github/workflows/tox.yml + .azure-devops/merge.yml — Python 3.11 and 3.12 end up with zero pre-merge unit coverage, because the two commits remove their only two homes.
Before this increment, 3.11/3.12 ran on every PR through the GitHub 12-combination matrix, and on the ADO Merge pipeline (run_unit_tests_ubuntu covered 3.10–3.13, run_unit_tests_macOs and run_unit_tests_windows covered 3.12). 4612e60a excludes them from the PR matrix and 67b1ecba deletes all three ADO jobs, so after this PR a 3.11- or 3.12-only regression is first observed on the post-merge push run against dev/preview/release/**. setup.py on this branch declares python_requires=">=3.10" with classifiers for 3.10, 3.11, 3.12 and 3.13, so that is half the supported interpreter set with no PR signal.
Cheapest fix that keeps the stated goal: add ubuntu-24.04 × 3.11 and ubuntu-24.04 × 3.12 back to the PR matrix (16 → 24 shard jobs, still half of the old 48). Alternatively run the full 12-combination matrix on a schedule: so drift is caught within a day instead of at merge.
.azure-devops/merge.yml (~line 43) — build_and_publish_azure_cli_test_sdk no longer has a consumer in this pipeline. The three deleted unit jobs were the only steps that pulled the test SDK (through templates/run-tests-parallel.yml). run_style_check still carries it in dependsOn, but its steps are setup-python → install-azure-cli-released → download-install-local-azure-iot-cli-extension-with-pip → pylint → flake8; none of those consume the published artifact. So Merge still spends a job building and publishing a wheel nothing downloads, and serialises the style check behind it.
CONTRIBUTING.md's new paragraph says Merge "retains ... the Azure CLI test SDK build", and test_ado_merge_retains_non_unit_checks_without_repeating_github_units pins the job set, so this reads as deliberate. Worth stating what still consumes it (another pipeline? the published feed?) or dropping the job and the dependsOn entry together.
Nits
azext_iot/tests/test_unit_shards_unit.py (~line 72) — test_pr_ci_does_not_duplicate_feature_branch_pushes models GitHub's branch filter with fnmatchcase, whose * crosses / while GitHub's does not (only ** does). The two agree for every pattern in the file today, so the test is correct as written, but the model would silently accept a future release/* as matching release/a/b. A one-line comment noting the approximation keeps the next editor honest.
Checked and clean
The exclude: expression evaluates correctly in both directions: true && '[...]' short-circuits to the JSON string, and false && '[...]' || '[]' yields '[]', so the full matrix is the default.
Shard arithmetic matches the docs exactly: 48 − 12 (3.12) − 12 (3.11) − 4 (windows 3.10) − 4 (macOS 3.10) = 16 unit jobs and 4 gate jobs on PRs; 48 and 12 everywhere else.
The two duplicated exclude: literals cannot drift: test_ci_shards_selected_os_python_combinations_without_mixing_artifacts builds a single matrix dict seeded from unit["strategy"]["matrix"]["exclude"] and asserts both the tox and unit-gate strategies against it.
Releases keep the full matrix. release_workflow.yml's unit-test: passes no with:, the input defaults to false, and test_only_ci_pull_requests_request_the_reduced_unit_matrix asserts exactly that for every tox.yml caller plus the absence of pr-matrix from workflow_dispatch.
1.1.0-preview really exists as a branch (confirmed through the branches API), so the literal in the new push: filter is not dead config, and release/** covers both release/1.0.0-preview and this PR's base release/1.1.0-preview.
tags: ["**"] preserves the tag-triggered CI that the previously unfiltered push: provided — narrowing branches: alone would have dropped it.
.azure-devops/templates/run-tests-parallel.yml still exists and is still referenced by templates/trigger-tests.yml, so removing its merge.yml callers leaves no dangling template.
The test_ado_dps_wiring_unit.py edit is a faithful collapse: the parametrize had exactly two values, one of which (merge.yml) no longer has trigger-tests.yml-style calls, and only the merge.yml-specific assertions were removed. pytest is still imported and used by other parametrized cases in the file.
CI-only change, so no HISTORY.rst entry or azext_iot/constants.pyVERSION bump is expected.
Not restating Copilot's open comment on azext_iot/tests/unit_test_durations.json (no measured duration for test_unit_shards_unit.py) — it is untouched by this increment.
This is an automated review and may be incomplete.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟢 Approval recommended
The shard validation is fail-closed, comprehensively tested, and supported by successful linked cross-platform and integration runs.
Review effort: Balanced Findings: None
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://github.com/Azure/azure-iot-cli-extension/actions/runs/37505979966
Runtime improvements
Observed successful runs across different revisions; timings vary.
Changes
Merge before #912.