[SPARK-58885][BUILD][INFRA] Refactor handling of ignored patterns in module test detection by nchammas · Pull Request #58136 · apache/spark

nchammas · 2026-08-19T19:46:12Z

What changes were proposed in this pull request?

Refactor how we ignore file patterns when determining what modules to test. Use pathlib.PurePath glob-style patterns instead of regexes.

Add some doctests to modules.py so we can test the new is_ignored_file function directly. Ensure these doctests can be run by themselves and are also run as part of run-tests.py.

Why are the changes needed?

#56312 captured ignored patterns as a dev-tools module that has special handling. The problem is that test module detection is additive, so a change to python/README.md triggers dev-tools and pyspark-install and pyspark-core. You can see this in the CI run associated with 75ac74b.

There is also a problem with using regexes for these ignore patterns because most of the patterns include extensions like .sh or .md without escaping the .. This leads to the unlikely but nonetheless surprising case of README1md matching the README.md pattern.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

I ran the doctests on modules.py and utils.py.

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

I wrote this with assistance from GitHub Copilot.

nchammas · 2026-08-19T20:11:11Z

+    "/LICENSE-binary",
+    "/NOTICE-binary",
+    "/scalastyle-config.xml",
+    "/SECURITY.md",


I think these could be loosened to match the pattern anywhere in the path hierarchy.

dongjoon-hyun

+1, LGTM

uros-b · 2026-08-20T22:06:39Z

+1, thank you @nchammas and @HyukjinKwon @dongjoon-hyun!

gaogaotiantian · 2026-08-21T00:12:07Z

+1, thanks! I think the PurePath approach is nice.

…module test detection Refactor how we ignore file patterns when determining what modules to test. Use `pathlib.PurePath` glob-style patterns instead of regexes. Add some doctests to `modules.py` so we can test the new `is_ignored_file` function directly. Ensure these doctests can be run by themselves and are also run as part of `run-tests.py`. [1]: https://github.com/apache/spark/actions/runs/32265494187/job/96146278679 There is also a problem with using regexes for these ignore patterns because most of the patterns include extensions like `.sh` or `.md` without escaping the `.`. This leads to the unlikely but nonetheless surprising case of `README1md` matching the `README.md` pattern. No. I ran the doctests on `modules.py` and `utils.py`. I wrote this with assistance from GitHub Copilot. Closes #58136 from nchammas/SPARK-58885-ignored-test-patterns. Authored-by: Nicholas Chammas <nicholas.chammas@gmail.com> Signed-off-by: Nicholas Chammas <nicholas.chammas@gmail.com> (cherry picked from commit 9a75a1d) Signed-off-by: Nicholas Chammas <nicholas.chammas@gmail.com>

nchammas · 2026-08-21T14:54:15Z

Merge Summary:

merged into master 9a75a1d
merged into branch-4.x 8cc56ea

Posted by merge_spark_pr.py

nchammas · 2026-08-21T15:12:04Z

Hmm, I had an interesting problem when cherry picking this onto branch-4.x. There was a merge conflict; I resolved it manually in my IDE (VS Code) and proceeded. The commit message stripped lines starting with #.

I think it did that because git cherry-pick --continue triggers the default commit.cleanup behavior which strips whitespace, unlike commit -m which preserves it. A manual fix would be for the operator to call git -c commit.cleanup=whitespace cherry-pick --continue, which would preserve whitespace.

nchammas · 2026-08-21T15:28:06Z

I've made a mistake and somehow pushed to master directly in 3731453. Apologies.

I think the culprit is a bad worktree setup that I did not notice. I believe it's because I let an agent create the worktree, and it pointed the worktree to upstream rather than to origin.

What's the correct protocol to fix this? Just revert and push directly?

cc @uros-b @HyukjinKwon @dongjoon-hyun @gaogaotiantian

dongjoon-hyun · 2026-08-21T15:56:36Z

Yes, the current protocol is to revert and push it directly ASAP, @nchammas . And, please don't give a commit permission to the agent. The first rule of my ~/CLAUDE.md is to disallow git commit.

nchammas · 2026-08-21T15:58:50Z

The agent didn't push in this case. It was me! I just didn't realize the worktree (created by the agent) was pointed at the wrong remote until it was too late. (For the record, I never let agents commit or push.)

I will revert and push right now.

nchammas · 2026-08-21T16:05:44Z

Reverted here: 976b87f

…trees or branches ### What changes were proposed in this pull request? Specify that when creating a new worktree or branch, agents should use `--no-track` so they don't accidentally target `upstream/master`. ### Why are the changes needed? I just pushed 3731453 directly to `master` from a new worktree. It was a mistake and I was surprised by it. The worktree was configured to push directly to `upstream/master`. I believe this happened because I let an agent (for the first time) create the worktree for me and it followed the instructions in our AGENTS file: > create a new git worktree from `<upstream>/master` and work from there More background here: #58136 (comment) To be clear, I made the commit and push myself, not the agent. What the agent did is create the worktree. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? In a fresh session I had Grok 4.5 create a new worktree using the updated instructions and it ran the following (edited for clarity): ``` git fetch upstream master git worktree add --no-track -b test-changes ../test-changes upstream/master git worktree list git -C ../test-changes status git -C ../test-changes log -1 --oneline ``` I then switched to the new worktree and confirmed it does not track any upstream: ``` $ git status -sb ## test-changes ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #58207 from nchammas/agents-worktrees. Authored-by: Nicholas Chammas <nicholas.chammas@gmail.com> Signed-off-by: Nicholas Chammas <nicholas.chammas@gmail.com>

### What changes were proposed in this pull request? When a conflict occurs on cherry picking a commit to a branch, ask the user to fix the conflict but have the script continue the cherry pick itself. ### Why are the changes needed? If the user calls `git cherry-pick --continue` themselves, commit message lines starting with `#` will be interpreted as comments and stripped. This is [what happened to me here][1]. Compare the commit message on 9a75a1d (master) to 8cc56ea (branch-4.x) and note how every line beginning with `#` was stripped from the latter. [1]: #58136 (comment) I'm not the only one who has hit this. It seems this has been happening for a while. #### Cherry-Pick Commit Message Loss Cases In all of these cases, the `branch-4x` commit messages are malformed relative to the originals from `master`. | Date | `master` source | `branch-4.x` backport | |---|---|---| | 2026-07-21 | [99025ce](99025ce) | [98e98b7](98e98b7) | | 2026-07-20 | [6b719b3](6b719b3) | [6c0a252](6c0a252) | | 2026-07-14 | [226340c](226340c) | [67df419](67df419) | | 2026-07-11 | [710b3c4](710b3c4) | [da117f6](da117f6) | | 2026-06-18 | [880083f](880083f) | [84fcfba](84fcfba) | | 2026-06-16 | [2fb4a1b](2fb4a1b) | [07d9f34](07d9f34) | | 2026-06-03 | [13b526d](13b526d) | [7a70689](7a70689) | | 2026-05-25 | [0af3d42](0af3d42) | [bdd5fdf](bdd5fdf) | | 2026-05-16 | [0a0d31b](0a0d31b) | [f5273c7](f5273c7) | | 2026-05-13 | [436291e](436291e) | [f67a855](f67a855) | | 2026-05-08 | [bb72aef](bb72aef) | [e7ae20a](e7ae20a) | ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? I used this test script to confirm that `commit.cleanup=scissors` preserves Markdown headings and other lines that begin with `#` when there is a cherry pick conflict. [test-cherry-pick-commit-msg.sh](https://github.com/user-attachments/files/31342347/test-cherry-pick-commit-msg.sh) I also tried the merge script from this branch with `--dry-run` and it worked, including a conflicted cherry pick to `branch-4.x`. ### Was this patch authored or co-authored using generative AI tooling? I wrote this with assistance from GitHub Copilot. Closes #58214 from nchammas/merge-pr-cherry-pick-whitespace. Authored-by: Nicholas Chammas <nicholas.chammas@gmail.com> Signed-off-by: Nicholas Chammas <nicholas.chammas@gmail.com>

nchammas added 2 commits August 19, 2026 15:36

refactor handling of ignored files

22b3010

add doctests directly to modules.py

8bfff49

nchammas commented Aug 19, 2026

View reviewed changes

formatting

d951c40

nchammas mentioned this pull request Aug 19, 2026

[SPARK-57254][INFRA] Put CI-unrelated files in a module so CI won't be triggered #56312

Closed

Merge branch 'master' into SPARK-58885-ignored-test-patterns

42e30cf

HyukjinKwon approved these changes Aug 19, 2026

View reviewed changes

Merge branch 'master' into SPARK-58885-ignored-test-patterns

4d00387

nchammas requested review from gaogaotiantian and uros-b August 20, 2026 13:35

dongjoon-hyun approved these changes Aug 20, 2026

View reviewed changes

uros-b approved these changes Aug 20, 2026

View reviewed changes

gaogaotiantian approved these changes Aug 21, 2026

View reviewed changes

nchammas closed this in 9a75a1d Aug 21, 2026

nchammas deleted the SPARK-58885-ignored-test-patterns branch August 21, 2026 15:12

nchammas mentioned this pull request Aug 21, 2026

[MINOR][INFRA] Instruct agents to use --no-track when creating worktrees or branches #58207

Closed

nchammas mentioned this pull request Aug 21, 2026

[SPARK-58944][INFRA] Complete cherry pick as part of merge script #58214

Closed

Navigation Menu

Uh oh!

[SPARK-58885][BUILD][INFRA] Refactor handling of ignored patterns in module test detection - #58136

[SPARK-58885][BUILD][INFRA] Refactor handling of ignored patterns in module test detection#58136
nchammas wants to merge 5 commits into
apache:masterfrom
nchammas:SPARK-58885-ignored-test-patterns

nchammas commented Aug 19, 2026 •

edited

Loading

Uh oh!

nchammas Aug 19, 2026

Uh oh!

dongjoon-hyun left a comment

Uh oh!

uros-b commented Aug 20, 2026

Uh oh!

gaogaotiantian commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026 •

edited

Loading

Uh oh!

dongjoon-hyun commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026 •

edited

Loading

Uh oh!

nchammas commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

nchammas commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

Uh oh!

nchammas Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

uros-b commented Aug 20, 2026

Uh oh!

gaogaotiantian commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Aug 21, 2026

Uh oh!

nchammas commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nchammas commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nchammas commented Aug 19, 2026 •

edited

Loading

nchammas commented Aug 21, 2026 •

edited

Loading

nchammas commented Aug 21, 2026 •

edited

Loading