Fix CI wrongly validating and testing unrelated integrations on stacked PRs#24543
Draft
vitkyrka wants to merge 2 commits into
Draft
Fix CI wrongly validating and testing unrelated integrations on stacked PRs#24543vitkyrka wants to merge 2 commits into
vitkyrka wants to merge 2 commits into
Conversation
…rations on stacked PRs - `files_changed()`/`content_changed()` in datadog_checks_dev's git tooling hardcoded a diff against `master`/`origin/master` instead of the actual PR base ref, unlike `get_base_ref()` (used elsewhere in the same module for per-file license header comparisons). When a PR is stacked on top of another unmerged branch, this makes every integration touched by the *base* branch look "changed", causing `ddev validate all changed` (run by run-validations.yml) to run full validations against unrelated integrations and spuriously fail things like license-headers. - compute-matrix.yml computed the test matrix's merge-base from the pull_request event's cached `base.sha`, which goes stale once the base branch (a moving/stacked feature branch) gets new commits without a new push to this PR's own head. That produces an overly broad diff and runs the full integration test matrix instead of just the integration actually touched by the PR. Fixed by diffing against the base branch name so GitHub resolves its tip live. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Validation ReportAll 21 validations passed. Show details
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: cac2ed1 | Docs | Datadog PR Page | Give us feedback! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
Fixes CI running the full integration test matrix (hundreds of jobs) instead of just the integration a PR actually touches, when the PR's base branch is itself another unmerged (stacked) branch — as seen on #24487, whose base is
vwhitchurch/docker-run-env-vars-e2e.Confirmed root cause:
compute-matrix.yml's "Fetch merge base" step computes the test matrix's diff base from thepull_requestevent's cachedbase.sha. That value is a snapshot taken the last time this PR's own head was synchronized — it does not update when the base branch itself receives new commits. For a PR stacked on a moving/WIP branch, that cachedbase.shacan go stale relative to the base branch's real tip, producing a much wider diff than the PR's own changes and triggering the full integration test matrix.I verified this directly against #24487's failing run (
29333935437): the "Fetch merge base" step usedbase.sha = 283cdf2(from 2026-07-10), four days older than the base branch's actual tip at run time, producing a huge diff (.github,datadog_checks_dev,gearmand,pulsar,ray,temporal) and running the full matrix. I then pushed a fresh commit to #24487's head (a diagnostic commit, reverted immediately after) — that refreshed the event'sbase.shato the base branch's current tip, and the very next run correctly scoped the matrix to justgearmand, with no code change needed. That confirms the staleness is the mechanism; this PR removes the dependency on that lucky-timing refresh by resolving the base branch live every run.Fixed by diffing against the base branch name (
github.event.pull_request.base.ref, percent-encoded) instead ofbase.sha, so GitHub always resolves its current tip rather than a cached value.Also included, as an independent correctness fix (not confirmed to have caused the specific validate failure on #24487 — see note below):
files_changed()/content_changed()indatadog_checks_dev/datadog_checks/dev/tooling/git.pyhardcoded a diff againstmaster/origin/masterinstead of the actual PR base ref, unlikeget_base_ref()(already defined in the same file) which correctly resolves it viaGITHUB_BASE_REF. This makes every integration touched by an unmerged base branch look "changed" forddev validate all changed, which is real and reproducible (confirmed via diagnostics on #24487), but in practice it didn't cause a validation failure there — the per-file license-header content comparison already uses the correct base ref and the affected files were identical, so it passed. Still, running full validations against integrations the PR never touched is wasteful and is a latent correctness gap, so it's fixed the same way (get_base_ref()).Motivation
PR #24487 intermittently ran the full integration test matrix instead of just
gearmand. Root-caused via a scratch worktree plus temporary, reverted diagnostic commits on #24487 (with the author's OK) to confirm the mechanism against live CI rather than guessing. Note: thelicense-headersfailure originally observed on #24487 does not appear to reproduce from the base-ref-selection issue — re-running validate on the same commit passed, so that specific failure looks like a one-off flake rather than something this PR'sfiles_changed()change addresses. This PR only changes diffing/base-ref logic; no code underdatadog_checks/is affected.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged