Additional groovy-groovysh tests

.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
 

.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.

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
 

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.

subprojects/groovy-groovysh/build.gradle

@@ -34,6 +34,8 @@ dependencies {
     implementation projects.groovyJson
     implementation projects.groovyNio
     testImplementation projects.groovyTest
+    testImplementation projects.groovyCsv         // for /slurp .csv default coverage
+    testImplementation projects.groovyTestJunit6  // for @ForkedJvm in CSV fallback tests
     implementation "net.java.dev.jna:jna:${versions.jna}"
     implementation "org.jline:jansi:${versions.jline}"
     implementation "org.jline:jline-builtins:${versions.jline}"
@@ -51,6 +53,16 @@ plugins.withId('eclipse') {
     }
 }
 
+// Forward -Djunit.network from the Gradle invocation to the test JVM so that
+// @EnabledIfSystemProperty in network-gated tests (e.g. SlurpCsvFallbackTest)
+// can see it.
+tasks.named('test') {
+    def network = System.getProperty('junit.network')
+    if (network) {
+        systemProperty 'junit.network', network
+    }
+}
+
 tasks.named('rat') {
     excludes << '**/jline/GroovyEngine.java' // BSD license as per NOTICE/LICENSE files
     excludes << '**/jline/ObjectInspector.groovy' // BSD license as per NOTICE/LICENSE files

subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Main.groovy

@@ -393,7 +393,7 @@ class Main {
                 }
                 name('groovysh')
             }.build()
-            if (terminal.width == 0 || terminal.height == 0) {
+            if (terminal.columns == 0 || terminal.rows == 0) {
                 terminal.size = new Size(120, 40) // hard-coded terminal size when redirecting
             }
             Thread executeThread = Thread.currentThread()
@@ -502,7 +502,7 @@ class Main {
                 println render(messages['startup_banner.1'])
                 println render(messages['startup_banner.2'])
             }
-            println '-' * (terminal.width - 1)
+            println '-' * (terminal.columns - 1)
 // for debugging
 //            def index = 0
 //            def lines = ['/slurp /Users/paulk/Projects/groovy/subprojects/groovy-json/src/test/resources/groovy9802.json',

subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/commands/ClassLoaderTest.groovy

@@ -0,0 +1,46 @@
+/*
+ *  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.
+ */
+package org.apache.groovy.groovysh.commands
+
+import org.junit.jupiter.api.Test
+
+/**
+ * Tests for the {@code /classloader} command.
+ *
+ * The command renders the engine's class loader using the {@code COLUMNS}
+ * option of {@code Printer.println(options, object)}; the support class's
+ * {@code DummyPrinter} extracts each column as a {@code name=value} entry
+ * so substring assertions remain straightforward.
+ */
+class ClassLoaderTest extends SystemTestSupport {
+
+    @Test
+    void viewExposesColumnDataForLoadedClassLoader() {
+        // Define a type so the class loader has at least one entry to render.
+        engine.execute('class ClassLoaderProbe {}')
+        system.execute('/classloader')
+        def out = printer.output.join()
+        // Each column name from the /classloader command appears as a
+        // `name=...` entry. Don't assert on values' exact shape (lists vary
+        // by JDK and previous test state); just confirm the columns rendered.
+        assert out.contains('loadedClasses=')
+        assert out.contains('definedPackages=')
+        assert out.contains('classPath=')
+    }
+}