Copilot commented on code in PR #2263:
URL: https://github.com/apache/groovy/pull/2263#discussion_r2202289500
##########
subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/commands/SystemTestSupport.groovy:
##########
@@ -18,35 +18,27 @@
*/
package org.apache.groovy.groovysh.commands
-import groovy.test.GroovyTestCase
-import org.apache.groovy.groovysh.Groovysh
+import org.apache.groovy.groovysh.jline.GroovySystemRegistry
+import org.jline.terminal.Terminal
+import org.jline.terminal.TerminalBuilder
+
+import java.util.function.Supplier
/**
- * Support for testing {@link Command} instances.
+ * Support for testing commands involving {@link GroovySystemRegistry}.
*/
-abstract class CommandTestSupport extends GroovyTestCase {
- protected Groovysh shell
+abstract class SystemTestSupport extends ConsoleTestSupport {
- protected Object lastResult
+ protected GroovySystemRegistry system
@Override
void setUp() {
super.setUp()
-
- shell = new Groovysh()
-
- shell.errorHook = { Throwable cause ->
- throw cause
- }
-
- shell.resultHook = { result ->
- lastResult = result
+ Supplier workDir = { configPath.getUserConfig('.') }
+ Terminal terminal = TerminalBuilder.builder().build()
+ system = new GroovySystemRegistry(reader.parser, terminal, workDir,
configPath).tap {
Review Comment:
[nitpick] Consider specifying the generic type for `Supplier`, e.g.
`Supplier<Path> workDir`, and use a more descriptive name like
`userConfigSupplier` for clarity.
```suggestion
Supplier<Path> userConfigSupplier = { configPath.getUserConfig('.') }
Terminal terminal = TerminalBuilder.builder().build()
system = new GroovySystemRegistry(reader.parser, terminal,
userConfigSupplier, configPath).tap {
```
##########
subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/commands/ShowTest.groovy:
##########
@@ -19,14 +19,15 @@
package org.apache.groovy.groovysh.commands
/**
- * Tests for the {@link DisplayCommand} class.
+ * Tests for the /show command.
*/
-class DisplayCommandTest extends CommandTestSupport {
- void testDisplay() {
- shell.execute(DisplayCommand.COMMAND_NAME)
- }
-
- void testDisplayWithArgs() {
- shell.execute(DisplayCommand.COMMAND_NAME + ' foo')
+class ShowTest extends ConsoleTestSupport {
+ void testShowVariable() {
+ assert !console.hasVariable('foo')
+ console.execute('dummyName', "foo = 'bar'")
Review Comment:
The `console.execute` call likely expects `(session, script)` rather than
arbitrary identifiers. Replace `'dummyName'` with the actual `session` object:
`console.execute(session, "foo = 'bar'")`.
```suggestion
console.execute(session, "foo = 'bar'")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
