Skip to content

Commit f5bcbec

Browse files
paulk-asertpaulk-asert
committed
groovysh add skills file
1 parent bc4cacc commit f5bcbec

4 files changed

Lines changed: 227 additions & 0 deletions

File tree

‎.agents/skills/groovy-tests/SKILL.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,21 @@ These are the recurring mistakes when working with Groovy tests:
6969
7. **Orphaned tagged regions.** A `// tag::...[] ... // end::...[]` block in `src/spec/test/` that no AsciiDoc file `include::`'s is dead weight. If you removed the include, remove the tagged region too.
7070
8. **`./gradlew test` as the inner loop.** It builds and runs the whole core suite. Use targeted runs (`:test --tests <FQN>` or `:<subproject>:test --tests <FQN>`) for development; reserve the full run for the final pre-PR check.
7171
9. **JDK-preview-dependent test in the wrong location.** Tests that need `--enable-preview` go in `subprojects/tests-preview/src/test/`, not core `src/test/`.
72+
10. **Using `String.valueOf(object)` in test assertions.** It calls Java's static `String.valueOf` and bypasses Groovy MetaClass dispatch — Maps render as `{k=v}` instead of `[k:v]`, and similar mismatches hit other Groovy-flavoured collections. Use `object.toString()` so the Groovy extensions apply. (`null.toString()` returns `'null'` in Groovy, so no separate null guard is needed.)
73+
11. **Locale-, platform-, or format-dependent assertions.** Don't bake JVM defaults into expected output — locale (number/date formatting), default timezone, line endings, file path separators, and default charset all vary across CI agents and contributor machines. Symptom: a test passes for the author and fails on a colleague's Windows box, or starts failing when CI rotates locales. Two patterns that bite repeatedly:
74+
- **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.
75+
- **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.
76+
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.
77+
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:
78+
79+
```groovy
80+
tasks.named('test') {
81+
def network = System.getProperty('junit.network')
82+
if (network) systemProperty 'junit.network', network
83+
}
84+
```
85+
86+
Without forwarding the gated test always skips, even with `-Djunit.network=true` on the Gradle CLI.
7287
7388
## Procedure for a JIRA regression test
7489

‎.agents/skills/groovysh/SKILL.md‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
<!--
2+
Licensed to the Apache Software Foundation (ASF) under one
3+
or more contributor license agreements. See the NOTICE file
4+
distributed with this work for additional information
5+
regarding copyright ownership. The ASF licenses this file
6+
to you under the Apache License, Version 2.0 (the
7+
"License"); you may not use this file except in compliance
8+
with the License. You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing,
13+
software distributed under the License is distributed on an
14+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
KIND, either express or implied. See the License for the
16+
specific language governing permissions and limitations
17+
under the License.
18+
-->
19+
---
20+
name: groovysh
21+
description: Guidance for changes in subprojects/groovy-groovysh/ — REPL command implementations, JLine integration, vendored fork files, and the layered terminal-aware test stack. Use when modifying anything in that subproject's tree, bumping the JLine version, or syncing the vendored fork files against upstream.
22+
license: Apache-2.0
23+
compatibility: claude, codex, copilot, cursor, gemini, aider
24+
metadata:
25+
audience: contributors to apache/groovy
26+
scope: subproject-groovy-groovysh
27+
---
28+
29+
# groovysh
30+
31+
Use this skill for work in the `subprojects/groovy-groovysh/` tree —
32+
the interactive Groovy REPL. The subproject is unusual in two ways:
33+
34+
1. It vendors files from JLine because we can't depend on the upstream
35+
`jline-groovy` artifact (circular dependency on Groovy itself).
36+
2. Its tests touch a real JLine `Terminal`, making them more
37+
platform-sensitive than the rest of the codebase.
38+
39+
This skill layers on top of [`groovy-tests`](../groovy-tests/SKILL.md) —
40+
load both together when adding tests for groovysh code.
41+
42+
## When to use this skill
43+
44+
**Use it for:**
45+
46+
- REPL command changes (`subprojects/groovy-groovysh/src/main/groovy/.../jline/`).
47+
- Anything terminal-related: writing tests, integrating JLine APIs, image rendering.
48+
- Bumping the JLine version in `versions.properties`.
49+
- Syncing vendored fork files against upstream JLine.
50+
51+
**Don't use it for:**
52+
53+
- Pure compiler/runtime changes elsewhere — use [`groovy-internals`](../groovy-internals/SKILL.md).
54+
- Changes to the project-wide build (root `build.gradle`, `build-logic/`) — use [`groovy-build`](../groovy-build/SKILL.md).
55+
56+
## Read first
57+
58+
- [`subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc`](../../../subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc) — user-facing reference, the source of truth for command behaviour.
59+
- The test support classes under `subprojects/groovy-groovysh/src/test/.../commands/`: `ConsoleTestSupport` (engine + console + printer) and `SystemTestSupport` (adds dumb terminal + system registry).
60+
61+
## Vendored JLine files
62+
63+
Five BSD-licensed files under `src/main/groovy/.../jline/` (`GroovyEngine.java`,
64+
`PackageHelper.java`, `JrtJavaBasePackages.java`, `ObjectInspector.groovy`,
65+
`Utils.groovy`) are forks of JLine sources, kept in-tree because the
66+
upstream artifact (`org.jline:jline-groovy`) depends on `org.apache.groovy:groovy`
67+
and would create a circular dependency. `GroovyPosixCommands.java` is
68+
similarly derived but diverged enough to be Apache-licensed.
69+
70+
If our customisations are merged upstream, the goal is to delete the
71+
in-tree forks and depend on the upstream artifact instead. Until then,
72+
treat the forks as code we own — but check the upstream version when
73+
bumping JLine, in case there are fixes worth picking up.
74+
75+
## Test layers
76+
77+
Three layers of decreasing portability — prefer the lowest one that
78+
demonstrates the property under test:
79+
80+
1. **Engine** — `GroovyEngine` directly; no terminal, no registry. See `GroovyEngineTest`.
81+
2. **Registry** — `GroovySystemRegistry` over a dumb terminal. See `GroovySystemRegistryTest`.
82+
3. **Command** — full `SystemTestSupport` stack. See `DelTest`, `ImportTest` for printer-based assertions; `HelpCommandTest` for the `terminalOutput()` capture pattern when a builtin writes through `terminal.writer()` instead of the printer.
83+
84+
## Top failure modes
85+
86+
1. **`TerminalBuilder.builder().build()` in a test.** Auto-detects the
87+
JVM's TTY and may probe native bindings. Use
88+
`TerminalBuilder.builder().dumb(true).streams(...).build()` instead;
89+
`SystemTestSupport` already does this.
90+
2. **Asserting on full terminal output strings.** Prefer
91+
`printer.output` — the `DummyPrinter` captures `object.toString()`,
92+
bypassing ANSI rendering. For JLine builtins that write through
93+
`terminal.writer()` instead (e.g. `/help`), use
94+
`SystemTestSupport.terminalOutput()` and assert on stable
95+
substrings, never on full-string compares. The dumb terminal still
96+
emits capability-probe escapes (`\e[?2027$p\e[c`) at startup, and
97+
JLine layout/spacing shifts between releases.
98+
3. **Treating the vendored forks as independent.** They are tightly
99+
coupled to the `GroovyEngine` deep fork — the small files reference
100+
`GroovyEngine.Format` etc. Don't delete one without re-deriving the
101+
coupling.
102+
4. **Confusing "we changed it" vs "upstream changed it".** When
103+
syncing forks, diff against the *fork-base* tag (the JLine version
104+
we originally forked from), not just current upstream. Otherwise
105+
our renames look like upstream additions.
106+
5. **Network/Maven tests without `-Djunit.network=true` gating.**
107+
`/grab` and similar pull from the network; they must be opt-in.
108+
6. **Hard-coded or implicit terminal width.** The dumb terminal
109+
reports columns/rows of 0. If a test cares, set
110+
`terminal.size = new Size(120, 40)` explicitly.
111+
7. **Calling `getWidth()`/`getHeight()` after a JLine bump.**
112+
Deprecated since JLine 4.x; use `getColumns()`/`getRows()`.
113+
8. **`/grep` and similar Posix commands emit ANSI match highlights
114+
by default.** The colour decision is per-command, not per-terminal,
115+
so a dumb terminal doesn't suppress it. When unit-testing, pass
116+
`--color=never` (or strip ANSI from the captured output) so
117+
substring assertions match contiguously.
118+
9. **Assuming `/save <file>` captures variable assignments.** It
119+
serialises `engine.buffer`, which in default mode includes only
120+
`IMPORT|TYPE|METHOD` snippets — bare variable assignments aren't
121+
there. Variables enter the buffer only when interpreter mode is
122+
enabled (`GROOVYSH_OPTIONS[INTERPRETER_MODE_PREFERENCE_KEY] = true`).
123+
The no-arg `/save` form is a separate path that JSON-serialises
124+
`engine.sharedData` and *does* include variables. Round-trip tests
125+
for the file-form should exercise definitions, not bare variables.
126+
127+
(Locale/platform/format brittleness is covered by the project-wide
128+
[`groovy-tests`](../groovy-tests/SKILL.md) skill — it's not unique to
129+
groovysh, though terminal-aware tests are particularly exposed.)
130+
131+
## Procedure for a JLine version bump
132+
133+
1. Bump `versions.properties:jline=...`.
134+
2. Run `:groovy-groovysh:test`.
135+
3. Compile-scan for new deprecation warnings; fix at the call site.
136+
4. Diff each vendored fork against the new upstream tag. Pick up
137+
substantive upstream fixes; skip cosmetic noise.
138+
5. Update this skill if any finding above changed.
139+
140+
## Validation checklist
141+
142+
Before declaring a groovysh change ready:
143+
144+
- [ ] Tests use `dumb(true).streams(...)` for any terminal they construct.
145+
- [ ] Output assertions use stable substrings, not full-string compares. Prefer `printer.output`; for terminal-side use `terminalOutput()`; for Posix commands use the context's `out` buffer with `--color=never` where applicable.
146+
- [ ] No locale-, platform-, or width-dependent assumptions.
147+
- [ ] `@AfterEach` closes any terminal the test constructed.
148+
- [ ] Network/Maven tests are gated by `-Djunit.network=true` or skipped.
149+
- [ ] No new calls to deprecated JLine APIs (`getWidth`/`getHeight`).
150+
- [ ] If a fork file was synced, the diff against fork-base distinguishes our changes from upstream.
151+
152+
## References
153+
154+
- [`subprojects/groovy-groovysh/AGENTS.md`](../../../subprojects/groovy-groovysh/AGENTS.md) — subproject pointer file that loads this skill.
155+
- [`subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc`](../../../subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc) — user-facing reference.
156+
- [`subprojects/groovy-groovysh/LICENSE`](../../../subprojects/groovy-groovysh/LICENSE) — provenance for the BSD-licensed vendored files.
157+
- [`groovy-tests`](../groovy-tests/SKILL.md) — sister skill for test conventions; load alongside this one.
158+
- [`AGENTS.md`](../../../AGENTS.md) — root agent guide.

‎AGENTS.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,21 @@ into the human-facing docs above.
127127

128128
| Skill | Use for |
129129
|---|---|
130+
| [`groovy-build`](.agents/skills/groovy-build/SKILL.md) | Gradle build changes — convention plugins, build files, dependency verification, ASM/ANTLR repackaging, OSGi, release pipeline |
130131
| [`groovy-internals`](.agents/skills/groovy-internals/SKILL.md) | Compiler and runtime work — parser, AST, type checker, transforms, class generation |
131132
| [`groovy-tests`](.agents/skills/groovy-tests/SKILL.md) | Adding or modifying tests, including JIRA regression tests and executable AsciiDoc examples |
133+
| [`groovysh`](.agents/skills/groovysh/SKILL.md) | Work in `subprojects/groovy-groovysh/` — REPL commands, JLine integration, vendored forks, terminal-aware test stack |
134+
135+
## Subproject guides
136+
137+
Some subprojects have their own `AGENTS.md` with content specific to
138+
that module — additional architecture, test infrastructure, or
139+
conventions that don't apply elsewhere. Load the relevant subproject's
140+
guide when working in its directory tree.
141+
142+
| Subproject | Scope |
143+
|---|---|
144+
| [`groovy-groovysh`](subprojects/groovy-groovysh/AGENTS.md) | Interactive REPL, JLine integration, vendored forks, terminal-aware test stack |
132145

133146
## Where to ask
134147

‎subprojects/groovy-groovysh/AGENTS.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<!--
2+
Licensed to the Apache Software Foundation (ASF) under one
3+
or more contributor license agreements. See the NOTICE file
4+
distributed with this work for additional information
5+
regarding copyright ownership. The ASF licenses this file
6+
to you under the Apache License, Version 2.0 (the
7+
"License"); you may not use this file except in compliance
8+
with the License. You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing,
13+
software distributed under the License is distributed on an
14+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
KIND, either express or implied. See the License for the
16+
specific language governing permissions and limitations
17+
under the License.
18+
-->
19+
20+
# Agent Guide for groovy-groovysh
21+
22+
Subproject-specific supplement to the [root `AGENTS.md`](../../AGENTS.md).
23+
24+
## What's special about this subproject
25+
26+
- groovysh is the interactive Groovy REPL, built on JLine 4.x.
27+
- It vendors a small set of files derived from JLine sources, plus a
28+
deep fork of `GroovyEngine.java` that we maintain ourselves.
29+
- Tests touch a real JLine `Terminal`, which makes them more
30+
platform-sensitive than the rest of the codebase.
31+
32+
For substantive guidance — what to read first, the vendored fork
33+
inventory, test layers, top failure modes, the JLine bump procedure,
34+
and the platform-fragility checklist — load the
35+
[`groovysh`](../../.agents/skills/groovysh/SKILL.md) skill.
36+
37+
## References
38+
39+
- [Root `AGENTS.md`](../../AGENTS.md) — licensing, commit conventions, project-wide rules.
40+
- [`src/spec/doc/groovysh.adoc`](src/spec/doc/groovysh.adoc) — user-facing reference.
41+
- [`LICENSE`](LICENSE) — provenance for the BSD-licensed vendored files.

0 commit comments

Comments
 (0)