alamb opened a new pull request, #27205: URL: https://github.com/apache/datafusion/pull/27205
## Which issue does this PR close? - N/A. Same change as https://github.com/apache/datafusion-sqlparser-rs/pull/2832 ## Rationale for this change The `cargo test (amd64)` job measures coverage and uploads it to Codecov. For a `pull_request` run, `actions/checkout` checks out the **merge** of the PR head into the base branch, but `codecov-action` uploads the report under the **PR head** SHA. Whenever the base branch has commits the PR lacks, Codecov reads the merge tree's coverage at the head's line numbers, so the per-line results in the Codecov comment and UI refer to the wrong lines. Today this affects `pull_request` runs in the `apache` repo: PRs to `branch-*` (backports) and non-fork PRs to `main`. Fork PRs to `main` run in the fork on `push` since #25591, where the head is already checked out, so they are not affected. Before #25591 every PR was affected, which is where the examples below come from. ### Examples of coverage results that refer to lines not in the PR For each PR below I took Codecov's per-line coverage for the head commit (via the Codecov API), the PR diff (merge-base..head) and the merge tree the job actually built (`git merge-tree` of the head and the `main` tip at the time the run was created). **#25455** (`datafusion/optimizer/src/push_down_filter.rs`): the [Codecov comment](https://github.com/apache/datafusion/pull/25455#issuecomment-5724280294) reports `70.73% | 15 Missing and 9 partials`. Among the lines it marks as *Missing* are [L4593-L4594, L4613-L4615](https://github.com/apache/datafusion/blob/406e428657115bbd268bf1ed13bd746a3080835e/datafusion/optimizer/src/push_down_filter.rs#L4593-L4615), which are a `///` doc comment, a blank line and two `//` comments: ```rust 4593: /// A filter is not moved below a pure extraction projection, even when its 4594: /// predicate only references pass-through columns. ... 4613: 4614: // `b` is a plain pass-through column, so without the precedence rule 4615: // this predicate would reach the scan as a `full_filters` entry. ``` `main` had 37 commits the PR lacked. In the measured merge tree those line numbers hold `TestUserNode::with_exprs_and_inputs`, a test helper from a different PR. 80 of the 167 added lines in that file have the wrong status. **#25696** (`datafusion/physical-plan/src/aggregates/mod.rs`): the [Codecov comment](https://github.com/apache/datafusion/pull/25696#issuecomment-5810707073) reports `63.59% | 60 Missing and 15 partials`. It shows the body of `with_limit_options` ([L1388-L1394](https://github.com/apache/datafusion/blob/0880e988132bc32ce227f627b6c9c4c55340c957/datafusion/physical-plan/src/aggregates/mod.rs#L1388-L1394)) as covered, while the lines actually measured at those numbers were the tail of `try_new` and `fn mode()`; the real `with_limit_options` body sits 14 lines further down in the merge tree and was never executed. Conversely [L1406-L1407](https://github.com/apache/datafusion/blob/0880e988132bc32ce227f627b6c9c4c55340c957/datafusion/physical-plan/src/aggregates/mod.rs#L1406-L1407) (a blank line and a `///` doc comment) are reported as *Missing*. `main` had 48 commits the PR lacked. **#25892** (`datafusion/expr/src/logical_plan/plan.rs`): the [Codecov comment](https://github.com/apache/datafusion/pull/25892#issuecomment-5908329882) reports `86.11% | 6 Missing and 4 partials`. [L5464-L5465](https://github.com/apache/datafusion/blob/a62969d5d4fa5a5cb23f4e55a35cd54bc7ba3bee/datafusion/expr/src/logical_plan/plan.rs#L5464-L5465), a blank line and the comment `// The two recursion outputs both replace the same list input.`, are reported as *Missing*. The lines measured at those numbers were `assert!(limit.get_skip_type().is_err());` from a test another PR had added to `main` (232 commits the PR lacked). 59 of the 116 added lines have the wrong status. **#26184** (`datafusion/optimizer/src/common_subexpr_eliminate.rs`): the [Codecov comment](https://github.com/apache/datafusion/pull/26184#issuecomment-6100374737) reports `80.85% | 9 Missing and 18 partials`. Only 10 commits separated the PR from `main`, but one of them removed 4 lines above the PR's changes, so 221 of the 225 added lines were read 4 lines off: 137 of them have the wrong status. For example [L2036](https://github.com/apache/datafusion/blob/b3004e5389b775e019b5e0a53dae70d249c80292/datafusion/optimizer/src/common_subexpr_eliminate.rs#L2036), a line inside a snapshot string literal, is reported as *Missing*, while the `?` lines next to it are reported as fully covered. Out of 243 PRs merged to `main` between 2026-09-25 and 2026-10-20 that touch Rust files, 26 had at least one changed file whose coverage was read at shifted line numbers. ## What changes are included in this PR? The coverage job's `actions/checkout` step now checks out `github.event.pull_request.head.sha` on `pull_request` runs, and `github.sha` otherwise (merge queue, pushes to `branch-*`), so the measured tree is the one Codecov attributes the report to. The trade-off is the same as in the sqlparser PR: the `cargo test (amd64)` job now tests the PR head rather than the merge with the base branch. The merge queue still tests the merged result before anything lands on `main`. ## What is the testing strategy for this PR? CI on this PR. The Codecov comment on this PR cannot show the difference since it changes no Rust code; the examples above were verified against Codecov's per-line data and the reconstructed merge trees. ## Are there any user-facing changes? No. -- 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]
