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
commit f5bcbecdd1e1d19c075f7595e6e5443277d9a3d2 Author: Paul King <[email protected]> AuthorDate: Sat May 9 07:08:27 2026 +1000 groovysh add skills file --- .agents/skills/groovy-tests/SKILL.md | 15 ++++ .agents/skills/groovysh/SKILL.md | 158 ++++++++++++++++++++++++++++++++++ AGENTS.md | 13 +++ subprojects/groovy-groovysh/AGENTS.md | 41 +++++++++ 4 files changed, 227 insertions(+) diff --git a/.agents/skills/groovy-tests/SKILL.md b/.agents/skills/groovy-tests/SKILL.md index c5dc111d5b..1479fdcb0f 100644 --- a/.agents/skills/groovy-tests/SKILL.md +++ b/.agents/skills/groovy-tests/SKILL.md @@ -69,6 +69,21 @@ These are the recurring mistakes when working with Groovy tests: 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. 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. 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/`. +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.) +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: + - **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: + + ```groovy + tasks.named('test') { + def network = System.getProperty('junit.network') + if (network) systemProperty 'junit.network', network + } + ``` + + Without forwarding the gated test always skips, even with `-Djunit.network=true` on the Gradle CLI. ## Procedure for a JIRA regression test diff --git a/.agents/skills/groovysh/SKILL.md b/.agents/skills/groovysh/SKILL.md new file mode 100644 index 0000000000..831e8b19a0 --- /dev/null +++ b/.agents/skills/groovysh/SKILL.md @@ -0,0 +1,158 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> +--- +name: groovysh +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. +license: Apache-2.0 +compatibility: claude, codex, copilot, cursor, gemini, aider +metadata: + audience: contributors to apache/groovy + scope: subproject-groovy-groovysh +--- + +# groovysh + +Use this skill for work in the `subprojects/groovy-groovysh/` tree — +the interactive Groovy REPL. The subproject is unusual in two ways: + +1. It vendors files from JLine because we can't depend on the upstream + `jline-groovy` artifact (circular dependency on Groovy itself). +2. Its tests touch a real JLine `Terminal`, making them more + platform-sensitive than the rest of the codebase. + +This skill layers on top of [`groovy-tests`](../groovy-tests/SKILL.md) — +load both together when adding tests for groovysh code. + +## When to use this skill + +**Use it for:** + +- REPL command changes (`subprojects/groovy-groovysh/src/main/groovy/.../jline/`). +- Anything terminal-related: writing tests, integrating JLine APIs, image rendering. +- Bumping the JLine version in `versions.properties`. +- Syncing vendored fork files against upstream JLine. + +**Don't use it for:** + +- Pure compiler/runtime changes elsewhere — use [`groovy-internals`](../groovy-internals/SKILL.md). +- Changes to the project-wide build (root `build.gradle`, `build-logic/`) — use [`groovy-build`](../groovy-build/SKILL.md). + +## Read first + +- [`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. +- The test support classes under `subprojects/groovy-groovysh/src/test/.../commands/`: `ConsoleTestSupport` (engine + console + printer) and `SystemTestSupport` (adds dumb terminal + system registry). + +## Vendored JLine files + +Five BSD-licensed files under `src/main/groovy/.../jline/` (`GroovyEngine.java`, +`PackageHelper.java`, `JrtJavaBasePackages.java`, `ObjectInspector.groovy`, +`Utils.groovy`) are forks of JLine sources, kept in-tree because the +upstream artifact (`org.jline:jline-groovy`) depends on `org.apache.groovy:groovy` +and would create a circular dependency. `GroovyPosixCommands.java` is +similarly derived but diverged enough to be Apache-licensed. + +If our customisations are merged upstream, the goal is to delete the +in-tree forks and depend on the upstream artifact instead. Until then, +treat the forks as code we own — but check the upstream version when +bumping JLine, in case there are fixes worth picking up. + +## Test layers + +Three layers of decreasing portability — prefer the lowest one that +demonstrates the property under test: + +1. **Engine** — `GroovyEngine` directly; no terminal, no registry. See `GroovyEngineTest`. +2. **Registry** — `GroovySystemRegistry` over a dumb terminal. See `GroovySystemRegistryTest`. +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. + +## Top failure modes + +1. **`TerminalBuilder.builder().build()` in a test.** Auto-detects the + JVM's TTY and may probe native bindings. Use + `TerminalBuilder.builder().dumb(true).streams(...).build()` instead; + `SystemTestSupport` already does this. +2. **Asserting on full terminal output strings.** Prefer + `printer.output` — the `DummyPrinter` captures `object.toString()`, + bypassing ANSI rendering. For JLine builtins that write through + `terminal.writer()` instead (e.g. `/help`), use + `SystemTestSupport.terminalOutput()` and assert on stable + substrings, never on full-string compares. The dumb terminal still + emits capability-probe escapes (`\e[?2027$p\e[c`) at startup, and + JLine layout/spacing shifts between releases. +3. **Treating the vendored forks as independent.** They are tightly + coupled to the `GroovyEngine` deep fork — the small files reference + `GroovyEngine.Format` etc. Don't delete one without re-deriving the + coupling. +4. **Confusing "we changed it" vs "upstream changed it".** When + syncing forks, diff against the *fork-base* tag (the JLine version + we originally forked from), not just current upstream. Otherwise + our renames look like upstream additions. +5. **Network/Maven tests without `-Djunit.network=true` gating.** + `/grab` and similar pull from the network; they must be opt-in. +6. **Hard-coded or implicit terminal width.** The dumb terminal + reports columns/rows of 0. If a test cares, set + `terminal.size = new Size(120, 40)` explicitly. +7. **Calling `getWidth()`/`getHeight()` after a JLine bump.** + Deprecated since JLine 4.x; use `getColumns()`/`getRows()`. +8. **`/grep` and similar Posix commands emit ANSI match highlights + by default.** The colour decision is per-command, not per-terminal, + so a dumb terminal doesn't suppress it. When unit-testing, pass + `--color=never` (or strip ANSI from the captured output) so + substring assertions match contiguously. +9. **Assuming `/save <file>` captures variable assignments.** It + serialises `engine.buffer`, which in default mode includes only + `IMPORT|TYPE|METHOD` snippets — bare variable assignments aren't + there. Variables enter the buffer only when interpreter mode is + enabled (`GROOVYSH_OPTIONS[INTERPRETER_MODE_PREFERENCE_KEY] = true`). + The no-arg `/save` form is a separate path that JSON-serialises + `engine.sharedData` and *does* include variables. Round-trip tests + for the file-form should exercise definitions, not bare variables. + +(Locale/platform/format brittleness is covered by the project-wide +[`groovy-tests`](../groovy-tests/SKILL.md) skill — it's not unique to +groovysh, though terminal-aware tests are particularly exposed.) + +## Procedure for a JLine version bump + +1. Bump `versions.properties:jline=...`. +2. Run `:groovy-groovysh:test`. +3. Compile-scan for new deprecation warnings; fix at the call site. +4. Diff each vendored fork against the new upstream tag. Pick up + substantive upstream fixes; skip cosmetic noise. +5. Update this skill if any finding above changed. + +## Validation checklist + +Before declaring a groovysh change ready: + +- [ ] Tests use `dumb(true).streams(...)` for any terminal they construct. +- [ ] 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. +- [ ] No locale-, platform-, or width-dependent assumptions. +- [ ] `@AfterEach` closes any terminal the test constructed. +- [ ] Network/Maven tests are gated by `-Djunit.network=true` or skipped. +- [ ] No new calls to deprecated JLine APIs (`getWidth`/`getHeight`). +- [ ] If a fork file was synced, the diff against fork-base distinguishes our changes from upstream. + +## References + +- [`subprojects/groovy-groovysh/AGENTS.md`](../../../subprojects/groovy-groovysh/AGENTS.md) — subproject pointer file that loads this skill. +- [`subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc`](../../../subprojects/groovy-groovysh/src/spec/doc/groovysh.adoc) — user-facing reference. +- [`subprojects/groovy-groovysh/LICENSE`](../../../subprojects/groovy-groovysh/LICENSE) — provenance for the BSD-licensed vendored files. +- [`groovy-tests`](../groovy-tests/SKILL.md) — sister skill for test conventions; load alongside this one. +- [`AGENTS.md`](../../../AGENTS.md) — root agent guide. diff --git a/AGENTS.md b/AGENTS.md index b19bac690c..a35f2094f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,8 +127,21 @@ into the human-facing docs above. | Skill | Use for | |---|---| +| [`groovy-build`](.agents/skills/groovy-build/SKILL.md) | Gradle build changes — convention plugins, build files, dependency verification, ASM/ANTLR repackaging, OSGi, release pipeline | | [`groovy-internals`](.agents/skills/groovy-internals/SKILL.md) | Compiler and runtime work — parser, AST, type checker, transforms, class generation | | [`groovy-tests`](.agents/skills/groovy-tests/SKILL.md) | Adding or modifying tests, including JIRA regression tests and executable AsciiDoc examples | +| [`groovysh`](.agents/skills/groovysh/SKILL.md) | Work in `subprojects/groovy-groovysh/` — REPL commands, JLine integration, vendored forks, terminal-aware test stack | + +## Subproject guides + +Some subprojects have their own `AGENTS.md` with content specific to +that module — additional architecture, test infrastructure, or +conventions that don't apply elsewhere. Load the relevant subproject's +guide when working in its directory tree. + +| Subproject | Scope | +|---|---| +| [`groovy-groovysh`](subprojects/groovy-groovysh/AGENTS.md) | Interactive REPL, JLine integration, vendored forks, terminal-aware test stack | ## Where to ask diff --git a/subprojects/groovy-groovysh/AGENTS.md b/subprojects/groovy-groovysh/AGENTS.md new file mode 100644 index 0000000000..afc82406e7 --- /dev/null +++ b/subprojects/groovy-groovysh/AGENTS.md @@ -0,0 +1,41 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +# Agent Guide for groovy-groovysh + +Subproject-specific supplement to the [root `AGENTS.md`](../../AGENTS.md). + +## What's special about this subproject + +- groovysh is the interactive Groovy REPL, built on JLine 4.x. +- It vendors a small set of files derived from JLine sources, plus a + deep fork of `GroovyEngine.java` that we maintain ourselves. +- Tests touch a real JLine `Terminal`, which makes them more + platform-sensitive than the rest of the codebase. + +For substantive guidance — what to read first, the vendored fork +inventory, test layers, top failure modes, the JLine bump procedure, +and the platform-fragility checklist — load the +[`groovysh`](../../.agents/skills/groovysh/SKILL.md) skill. + +## References + +- [Root `AGENTS.md`](../../AGENTS.md) — licensing, commit conventions, project-wide rules. +- [`src/spec/doc/groovysh.adoc`](src/spec/doc/groovysh.adoc) — user-facing reference. +- [`LICENSE`](LICENSE) — provenance for the BSD-licensed vendored files.
