ADFA-5244: Make CI able to catch the Spotless/source-tree failure

[davidschachterADFA]

Three files. A CI step that can see the failure, a comment that says what the excludes actually do, and one .gitignore line.

The bug

tooling-api-model:copyToTestDir wrote a jar into the source tree (tests/test-home/.cg/init/model.jar), and root spotlessJava targets fileTree(rootDir). Gradle saw Spotless consuming another task's output and failed validation:

A problem was found with the configuration of task ':spotlessJava'
  Gradle detected a problem with the following location: '<root>/tests/test-home'
  Reason: Task ':spotlessJava' uses this output of task
  ':subprojects:tooling-api-model:copyToTestDir' without declaring ... a dependency.

The task itself is gone — ADFA-5263 (#1739) deleted it, having established that nothing reads the jar. What's left here is making sure the class of failure can't come back unseen.

Why CI couldn't see it

spotlessCheck alone always passed. copyToTestDir never entered that graph, so the validation never ran. It failed only when a compile task shared the invocation — which is exactly what you run to verify a change before pushing. So it surfaced the moment you touched code and looked like your change had broken formatting. It cost me a detour confirming an unrelated branch was clean.

The workflow now runs :common:compileV8DebugKotlin spotlessCheck together. Verified at the branch point 9c8f21777, where copyToTestDir still existed: that invocation fails with the error above, and passes at this branch's tip. The task pairing is the point of the step, so there's a comment on it saying why it must not be split back apart.

The exclude comment

traversalExcludes had a comment explaining that Gradle prunes a subtree only when an exclude matches the directory node itself — dir/** matches the contents instead and forces a descend-and-filter. True, but it omitted the half that bites: bare names are root-anchored. "flox" prunes rootDir/flox and nothing else, which is why the three .-prefixed entries carry **/. Reading "**/.gradle" as equivalent to ".gradle" and dropping the prefix would stop pruning every per-project cache dir and bring back the ADFA-4816 12-minute spotlessCheck. The comment now says both halves.

No exclude is added. An earlier revision of this branch excluded tests/; once #1739 removed the thing that wrote there, the exclude was hiding nothing and was dropped.

The .gitignore line

#1739 removed both tests/ entries. tests/**/.cg/init/model.jar is correctly gone — the task that wrote it is deleted. tests/test-home should have stayed: writeInitScript() in gradle-plugin/src/test/java/com/itsaky/androidide/gradle/utils.kt:86 resolves FileProvider.testHomeDir() and creates .cg/init/androidide.init.gradle there on every run of :gradle-plugin:test. Without the line, a test run leaves ?? tests/ in git status, in a directory where nothing is tracked.

That is a test writing at execution time, not a task declaring an output, so it never trips the validation this branch is about — it just leaves the tree dirty. Restored, and moved out from under the "Generated files for tooling API" header, which heads the composite-build output dirs rather than this.

Verified

• The previously failing invocation now succeeds, and demonstrably failed at 9c8f21777.
• Nothing is tracked under tests/ (git ls-files tests/ is empty), so ignoring it hides no real source.
• Negative test: injecting a 4-space indent into build.gradle.kts still fails spotlessKotlinGradleCheck, and removing it passes. The check is still doing its job, not silently disabled.
• Spotless does not target *.gradle, so the generated init script was never at risk of being format-checked.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fbb29a8d-dfbe-4ca7-a312-11b9cfa1ed58

📥 Commits

Reviewing files that changed from the base of the PR and between 9c8f217 and 32975f5.

📒 Files selected for processing (1)

• build.gradle.kts

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

• Exclude generated tests/test-home artifacts from root-level Spotless traversal.
• Prevent Gradle task validation failures during combined compilation and spotlessCheck runs.
• Verify successful compilation and formatting checks.
• Verify that Spotless still detects violations outside the excluded directory.
• Risk: Formatting checks do not cover files under tests/test-home. This is intentional because the directory contains generated test artifacts.

Walkthrough

The Spotless configuration excludes tests/test-home from traversal. This prevents generated tooling API test artifacts from entering formatting checks.

Changes

Formatting traversal

Layer / File(s)	Summary
Exclude generated test artifacts build.gradle.kts	Spotless now prunes tests/test-home during formatting traversal.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 32975

This localized build configuration change excludes generated test-home content from formatting traversal while preserving the existing formatting checks; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: itsaky-adfa, hal-eisen-adfa, jomen-adfa

Poem

A rabbit checks the build with care,
No test-home jars are formatted there.
Spotless hops past generated hay,
Keeping checks neat through the day.
Nibble, hop, hooray!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title describes the CI objective of exposing the Spotless/source-tree failure. It is related to the changeset, although it does not mention the specific tests/test-home exclusion.
Description check	✅ Passed	The description directly explains the Spotless traversal issue, the CI verification change, the exclusion rationale, and the .gitignore update. It is clearly related to the changeset.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch task/ADFA-5244-spotless-testhome

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[davidschachterADFA]

The exclude is gone; this PR is now the CI guard plus a corrected comment

Reviewing this PR is what surfaced the actual cause: copyToTestDir copied a jar nothing reads into tests/test-home inside the source tree, from a configuration-time mkdir. That task is deleted in #1739 (ADFA-5263), so nothing creates the directory and there is nothing left to prune. Keeping the exclude would have left a permanent workaround for a problem that no longer exists.

This PR is therefore rebased onto #1739 and its base retargeted there, because the two are genuinely coupled: I removed the exclude while still based on stage and :common:compileV8DebugKotlin spotlessCheck failed immediately — with copyToTestDir still present and no exclude, the failure returns. Stacked on #1739 the same command passes. Verified both ways, which is also why merging this ahead of #1739 would have turned its own new CI step red.

What remains, and why each part earns its place

• The CI step runs a compile task in the same invocation as spotlessCheck. CI previously ran spotlessCheck standalone — the one invocation that can never pull a source-tree-writing task into the graph, so this entire class of failure was invisible to CI by construction. Worth keeping regardless of copyToTestDir, since any future task with a source-tree output reintroduces it.
• The corrected comment above traversalExcludes. The old text said "bare dir names required"; the actual rule is that the pattern must match the directory node, rooted or bare — and a bare name matches that name at any depth, so test-home would also have pruned testing/resources/test-home, the directory FileProvider.testHomeDir() actually points at.

Not related, despite the name

ADFA-5265 ("Spotless is slow") was a separate ticket of mine and turned out to be a misdiagnosis — measured at 6.5s warm / 22s cold, against 20s for configuration alone. Closed. This ticket was about Spotless failing the build, which was entirely real.

[davidschachterADFA]

Reviewed at xhigh. One fix pushed in 6c7a73f; two things about the PR as presented.

Fixed: the .gitignore deletion goes one line too far

The model.jar entry is genuinely dead — ADFA-5263 deleted the task that wrote it. tests/test-home is not. writeInitScript() in gradle-plugin/src/test/java/com/itsaky/androidide/gradle/utils.kt:86 resolves FileProvider.testHomeDir() and creates .cg/init/androidide.init.gradle on every run of :gradle-plugin:test. Verified on this branch:

$ touch tests/test-home/.cg/init/androidide.init.gradle
$ git check-ignore -v tests/test-home/.cg/init/androidide.init.gradle   # no match
$ git status --short
?? tests/

That is a test writing at execution time, not a task declaring an output, so it never trips the implicit-dependency validation this branch is about — it just leaves the tree dirty after a test run, in a directory where nothing is tracked. Restored the one line.

The copyToTestDir deletion is already in stage

059629232 (#1739) landed it. f2bafe28e here is a second copy — it will merge cleanly, but a third of the diff is a change reviewers have already reviewed. Worth merging stage in so what's left is the two lines that are actually new.

The description no longer matches the diff

The body's "The fix" section says traversalExcludes gains a tests/test-home entry. Commit 3d62974ac dropped that, and build.gradle.kts is now a comment-only change. The body also argues the prune is safe because "tests/test-home is gitignored generated content" — which was the same PR's reason for un-ignoring it. Worth a rewrite before merge.

Verified good

The workflow's task choice does reproduce the failure. At the branch point 9c8f21777, both :common:compileV8DebugKotlin spotlessCheck (what the workflow runs) and :common:compileV8DebugAndroidTestKotlin spotlessCheck (what the body cites) fail with Task ':spotlessJava' uses this output of task ':subprojects:tooling-api-model:copyToTestDir' without declaring ... a dependency, and both pass at the tip. The new comment on traversalExcludes is accurate about root-anchoring, and spotless doesn't target *.gradle, so the generated init script was never at risk of being format-checked.

[davidschachterADFA]

Correction to my review above: I attributed the .gitignore removal to this PR. It isn't from here.

origin/stage already carries it — 059629232 (ADFA-5263, #1739) deleted both lines, and git show origin/stage:.gitignore | grep tests returns nothing. This branch predates that merge and carries its own copy, so gh pr diff shows the deletion against the merge base and I read it as new. The regression is real and is in stage today; it just wasn't introduced here.

6c7a73fb1 still stands — it restores tests/test-home, which :gradle-plugin:test refills on every run via writeInitScript(). It is now a fix to stage's state that happens to ride on this branch rather than a fix to this branch. Moving it to its own PR against stage would be cleaner if you'd rather this one stay narrow; say the word and I'll split it.

The rest of the review is unaffected: the copyToTestDir deletion here duplicates 059629232, the body still describes a traversalExcludes entry the diff no longer has, and the CI task choice does reproduce the failure at 9c8f21777.

One more thing that a stage merge would settle: this branch also predates 113b57f92 (ADFA-5264, #1740), so a two-dot diff shows it dropping testing/resources/test-project/.cg/ from .gitignore. A three-way merge will keep stage's line, so nothing is actually reverted — but the diff reads alarmingly until the merge happens.

[davidschachterADFA]

Merged stage in (aaaed07e3) and rewrote the description.

The diff is now the three files this PR is actually about — +18/-5, down from 60 files. ADFA-5263 (#1739) and ADFA-5264 (#1740) had both landed on stage independently, so a third of the old diff was a duplicate copyToTestDir deletion and the rest was staleness (including testing/resources/test-project/.cg/ reading as a deletion, which a three-way merge never would have applied).

One conflict, in .gitignore: stage removed both tests/ entries via #1739. Kept this branch's tests/test-home, left model.jar deleted, and moved the entry out from under the "Generated files for tooling API" header — that heads the composite-build output dirs, not this.

The description now describes what the diff does rather than the traversalExcludes approach the branch abandoned in 3d62974ac. Re-verified post-merge: :common:compileV8DebugKotlin spotlessCheck passes at the tip, tests/test-home/.cg/init/androidide.init.gradle is ignored again, and stage's ADFA-5264 rule survived the conflict resolution.