This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 77972c8366 minor refactor: skills tweaks
77972c8366 is described below
commit 77972c8366bd0c0943e3e556ce824390459ce05e
Author: Paul King <[email protected]>
AuthorDate: Tue May 12 08:17:11 2026 +1000
minor refactor: skills tweaks
---
.agents/skills/groovy-internals/SKILL.md | 3 ++-
.agents/skills/groovy-tests/SKILL.md | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/.agents/skills/groovy-internals/SKILL.md
b/.agents/skills/groovy-internals/SKILL.md
index 4d127c2e17..0bfd7fe43f 100644
--- a/.agents/skills/groovy-internals/SKILL.md
+++ b/.agents/skills/groovy-internals/SKILL.md
@@ -66,7 +66,8 @@ work. Each one is cheap to avoid and expensive to land:
4. **Picking the wrong compile phase for a transform.** Read `CompilePhase`
and pick the earliest phase where the AST is in the state your transform needs.
Local transforms default to `CANONICALIZATION`; transforms that need resolved
types should run at `INSTRUCTION_SELECTION` or later.
5. **Reformatting code outside the change.** This project's review culture
rejects drive-by reformatting. Match the surrounding file's indentation, brace
style, and import order; do not run a formatter over files you didn't otherwise
touch.
6. **Adding a new public API surface "for completeness".** Public API in this
codebase is covenanted and hard to remove — see
`subprojects/binary-compatibility/`. New API needs a discussion, not just a PR.
-7. **Using wildcard imports.** Match the explicit-import convention already in
the tree.
+7. **Silently widening visibility when refactoring a Groovy helper.** Groovy's
default method visibility is **public**, not package-private. Dropping
`private` from `private static foo()` to make it a `static` helper makes it
part of the public API — `static foo()` is `public static foo()`. For "visible
for testing" helpers in `.groovy` files, use `@groovy.transform.PackageScope`
so same-package tests see the method while external callers don't. The existing
helpers in `groovy.grape.ivy. [...]
+8. **Using wildcard imports.** Match the explicit-import convention already in
the tree.
8. **Skipping the regression test.** Every bug fix that has a JIRA needs a
test that fails on master and passes after the fix. The naming convention is in
[`groovy-tests`](../groovy-tests/SKILL.md) — standalone classes use
`Groovy<NNNN>`; regressions added to an existing class get a `// GROOVY-<NNNN>`
comment so the JIRA stays searchable.
## Procedure
diff --git a/.agents/skills/groovy-tests/SKILL.md
b/.agents/skills/groovy-tests/SKILL.md
index 1479fdcb0f..fe5892e13d 100644
--- a/.agents/skills/groovy-tests/SKILL.md
+++ b/.agents/skills/groovy-tests/SKILL.md
@@ -74,7 +74,8 @@ These are the recurring mistakes when working with Groovy
tests:
- **Path strings interpolated into a parsed command line.** A
Windows-native `Path.toString()` like `C:\Users\…\foo.json` interpolated into a
`system.execute("cmd ${file}")`-style line gets its backslashes eaten by
JLine's `DefaultParser` (which treats `\` as an escape). Forward-slash the path
before interpolating: `path.toString().replace('\\', '/')`. Java NIO accepts
forward-slash paths on Windows.
- **Output captured from `PrintStream.println`.** `println` uses
`System.lineSeparator()`, which is `\r\n` on Windows. Line-aware assertions
(`output.split('\n')`, `output.contains('foo\n')`) silently fail on Windows.
Use Groovy's `String.normalize()` extension to collapse platform line
separators to `\n` before splitting/comparing.
Other defences: `Locale.ROOT` for date/number formatting, explicit
`StandardCharsets.UTF_8` rather than the platform default, or assert on parsed
values rather than their stringified forms.
-12. **`-Djunit.network` doesn't reach the test JVM by default.** The
build-logic uses it at the source-set filter level (excluding `groovy/grape/`
paths from compilation when unset). If you need the property visible at runtime
— for example to gate a non-Grape network test via
`@EnabledIfSystemProperty(named = 'junit.network', matches = 'true')` — the
subproject's `build.gradle` has to forward it:
+12. **`-Djunit.network=true` is required for any test under `groovy/grape/`.**
Symptom: `:groovy-grape-*:test --tests <FQN>` reports `BUILD SUCCESSFUL` but no
test results appear (and `--rerun-tasks` reports `NO-SOURCE`). The `Test` task
in `org.apache.groovy-tested.gradle` applies an `exclude
buildExcludeFilter(...)` filter (lines ~136, 265-278) that drops anything under
`groovy/grape/` from execution unless `junit.network` is set. The test classes
compile normally; they just aren't run [...]
+13. **`-Djunit.network` set on the Gradle CLI doesn't reach the test JVM
automatically.** Separate from the source-set filter above: if a test reads the
property at runtime — e.g. gated via `@EnabledIfSystemProperty(named =
'junit.network', matches = 'true')` — the subproject's `build.gradle` has to
forward it explicitly:
```groovy
tasks.named('test') {
