andygrove opened a new pull request, #5850:
URL: https://github.com/apache/datafusion-comet/pull/5850
## Which issue does this PR close?
Part of #5838. Pure refactor — no behaviour change.
## Rationale for this change
Every heavy job in `ci.yml` carries a four-line `${{ }}` expression that
ANDs together a path-filter output, an event-name test, an opt-in label test,
and a special case for `labeled` events:
```yaml
if: |
needs.changes.outputs.spark_3_4 == 'true' &&
(github.event_name == 'push' ||
github.event_name == 'workflow_dispatch' ||
(github.event_name == 'pull_request' &&
contains(github.event.pull_request.labels.*.name, 'run-spark-3.4-tests')
&&
(github.event.action != 'labeled' ||
github.event.label.name == 'run-spark-3.4-tests')))
```
Ten jobs, ten near-identical copies. Half the routing policy already lives
in `compute-changes.py`; this is the other half, expressed in a language that
can't be tested and has to be evaluated by hand to review.
I noticed it while sizing up the merge-queue work in #5838 — that change
would have meant editing all ten of these. It's cheaper to fix the shape first.
## What changes are included in this PR?
The event/label policy moves into `dev/ci/compute-changes.py`, next to the
path filters it was already being ANDed with. Each gate becomes:
```yaml
if: needs.changes.outputs.spark_3_4 == 'true'
```
and the routing is one table:
```python
POLICY = {
"spark_3_5": ["pr", "push"],
"spark_3_4": ["push", "label:run-spark-3.4-tests"],
...
}
```
`ci.yml` loses 62 lines net.
The real payoff is that the policy is now testable. `check-ci-config.py`
grows `POLICY_CASES`, pinning the expected job set for each event shape —
including that a non-gating label like dependabot's `dependencies` starts
nothing, which is #5007 and was previously only enforceable by reading YAML
very carefully.
One thing that fell out of writing the tests: `"pr"` alongside a `"label:"`
tier reads as "runs on every PR, and also when labelled", but the label check
wins and the `"pr"` is dead. `check-ci-config.py` now rejects that combination
rather than letting it be a silent no-op.
## How are these changes tested?
Differentially, against the code being replaced. I extracted the
pre-refactor `if:` expressions from `ci.yml` at the merge base, translated them
to Python, and evaluated them against the new `POLICY` over every combination
of event name, `pull_request` action, label set and added label — **1464 (job,
event) pairs, zero mismatches**.
To confirm the harness has teeth, three seeded regressions are each caught:
renaming a gating label (32 mismatches), dropping a label gate entirely (32),
and dropping the `labeled`-event narrowing that #5007 was about (105).
`POLICY_CASES` is the part that ships, since the differential harness needs
the old YAML to compare against. `actionlint` and `prettier --check "**/*.md"`
are clean.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
