[
https://issues.apache.org/jira/browse/GEODE-3788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16261052#comment-16261052
]
ASF GitHub Bot commented on GEODE-3788:
---------------------------------------
jinmeiliao closed pull request #1082: GEODE-3788: GfshParserRule enhancement
URL: https://github.com/apache/geode/pull/1082
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
index f8cd315f1f..d4dc4e8e57 100644
---
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
+++
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
@@ -29,7 +29,6 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
-import org.junit.rules.ExpectedException;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
@@ -45,9 +44,6 @@
@Rule
public GfshParserRule commandRule = new GfshParserRule();
- @Rule
- public ExpectedException thrown = ExpectedException.none();
-
private StartLocatorCommand spy;
@Before
@@ -58,7 +54,6 @@ public void before() throws Exception {
@Test
public void startLocatorWorksWithNoOptions() throws Exception {
- thrown.expect(NullPointerException.class);
commandRule.executeCommandWithInstance(spy, "start locator");
ArgumentCaptor<Properties> gemfirePropertiesCaptor =
ArgumentCaptor.forClass(Properties.class);
@@ -75,7 +70,6 @@ public void
startLocatorRespectsJmxManagerHostnameForClients() throws Exception
String startLocatorCommand = new CommandStringBuilder("start locator")
.addOption(JMX_MANAGER_HOSTNAME_FOR_CLIENTS, FAKE_HOSTNAME).toString();
- thrown.expect(NullPointerException.class);
commandRule.executeCommandWithInstance(spy, startLocatorCommand);
ArgumentCaptor<Properties> gemfirePropertiesCaptor =
ArgumentCaptor.forClass(Properties.class);
diff --git
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
index 92f874993a..1e6b7c2d4d 100644
---
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
+++
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
@@ -29,7 +29,6 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
-import org.junit.rules.ExpectedException;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
@@ -45,9 +44,6 @@
@Rule
public GfshParserRule commandRule = new GfshParserRule();
- @Rule
- public ExpectedException thrown = ExpectedException.none();
-
private StartServerCommand spy;
@Before
@@ -58,7 +54,6 @@ public void before() throws Exception {
@Test
public void startServerWorksWithNoOptions() throws Exception {
- thrown.expect(NullPointerException.class);
commandRule.executeCommandWithInstance(spy, "start server");
ArgumentCaptor<Properties> gemfirePropertiesCaptor =
ArgumentCaptor.forClass(Properties.class);
@@ -75,7 +70,6 @@ public void startServerRespectsJmxManagerHostnameForClients()
throws Exception {
String startServerCommand = new CommandStringBuilder("start server")
.addOption(JMX_MANAGER_HOSTNAME_FOR_CLIENTS, FAKE_HOSTNAME).toString();
- thrown.expect(NullPointerException.class);
commandRule.executeCommandWithInstance(spy, startServerCommand);
ArgumentCaptor<Properties> gemfirePropertiesCaptor =
ArgumentCaptor.forClass(Properties.class);
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
index d4f52187b8..5a165ea05f 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
@@ -34,9 +34,15 @@
public class CommandExecutor {
private Logger logger = LogService.getLogger();
+ // used by the product
public Object execute(ParseResult parseResult) {
+ return execute(null, parseResult);
+ }
+
+ // used by the GfshParserRule to pass in a mock command
+ public Object execute(Object command, ParseResult parseResult) {
try {
- Object result = invokeCommand(parseResult);
+ Object result = invokeCommand(command, parseResult);
if (result == null) {
return ResultBuilder.createGemFireErrorResult("Command returned null:
" + parseResult);
@@ -83,9 +89,12 @@ public Object execute(ParseResult parseResult) {
}
}
- protected Object invokeCommand(ParseResult parseResult) {
- return ReflectionUtils.invokeMethod(parseResult.getMethod(),
parseResult.getInstance(),
+ protected Object invokeCommand(Object command, ParseResult parseResult) {
+ // if no command instance is passed in, use the one in the parseResult
+ if (command == null) {
+ command = parseResult.getInstance();
+ }
+ return ReflectionUtils.invokeMethod(parseResult.getMethod(), command,
parseResult.getArguments());
}
-
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
index f4352b253f..12f43efee0 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
@@ -15,7 +15,6 @@
package org.apache.geode.management.internal.cli.commands;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
@@ -122,9 +121,9 @@ public void multipleResultReturnedWithOneException() throws
Exception {
when(result2.isSuccessful()).thenReturn(false);
when(result2.getThrowable()).thenReturn(new
IllegalArgumentException("something happened"));
- assertThatThrownBy(
- () -> parser.executeCommandWithInstance(command, "destroy region
--name=test"))
- .isInstanceOf(IllegalArgumentException.class);
+ parser.executeAndAssertThat(command, "destroy region
--name=test").statusIsError()
+ .containsOutput("something happened");
+
// verify that xmlEntiry returned by the result1 is not saved to Cluster
config
verify(command, never()).persistClusterConfiguration(any(), any());
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/CommandExecutorTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/CommandExecutorTest.java
index 4f51f25008..85001dedb1 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/CommandExecutorTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/CommandExecutorTest.java
@@ -58,14 +58,14 @@ public void executeWhenGivenDummyParseResult() throws
Exception {
@Test
public void returnsResultAsExpected() throws Exception {
- doReturn(result).when(executor).invokeCommand(any());
+ doReturn(result).when(executor).invokeCommand(any(), any());
Object thisResult = executor.execute(parseResult);
assertThat(thisResult).isSameAs(result);
}
@Test
public void testNullResult() throws Exception {
- doReturn(null).when(executor).invokeCommand(any());
+ doReturn(null).when(executor).invokeCommand(any(), any());
Object thisResult = executor.execute(parseResult);
assertThat(thisResult.toString()).contains("Command returned null");
}
@@ -73,7 +73,7 @@ public void testNullResult() throws Exception {
@Test
public void anyRuntimeExceptionGetsCaught() throws Exception {
;
- doThrow(new RuntimeException("my message
here")).when(executor).invokeCommand(any());
+ doThrow(new RuntimeException("my message
here")).when(executor).invokeCommand(any(), any());
Object thisResult = executor.execute(parseResult);
assertThat(((CommandResult)
thisResult).getStatus()).isEqualTo(Result.Status.ERROR);
assertThat(thisResult.toString()).contains("my message here");
@@ -81,7 +81,8 @@ public void anyRuntimeExceptionGetsCaught() throws Exception {
@Test
public void notAuthorizedExceptionGetsThrown() throws Exception {
- doThrow(new NotAuthorizedException("Not
Authorized")).when(executor).invokeCommand(any());
+ doThrow(new NotAuthorizedException("Not
Authorized")).when(executor).invokeCommand(any(),
+ any());
assertThatThrownBy(() -> executor.execute(parseResult))
.isInstanceOf(NotAuthorizedException.class);
}
@@ -89,7 +90,8 @@ public void notAuthorizedExceptionGetsThrown() throws
Exception {
@Test
public void anyIllegalArgumentExceptionGetsCaught() throws Exception {
;
- doThrow(new IllegalArgumentException("my message
here")).when(executor).invokeCommand(any());
+ doThrow(new IllegalArgumentException("my message
here")).when(executor).invokeCommand(any(),
+ any());
Object thisResult = executor.execute(parseResult);
assertThat(((CommandResult)
thisResult).getStatus()).isEqualTo(Result.Status.ERROR);
assertThat(thisResult.toString()).contains("my message here");
@@ -98,7 +100,8 @@ public void anyIllegalArgumentExceptionGetsCaught() throws
Exception {
@Test
public void anyIllegalStateExceptionGetsCaught() throws Exception {
;
- doThrow(new IllegalStateException("my message
here")).when(executor).invokeCommand(any());
+ doThrow(new IllegalStateException("my message
here")).when(executor).invokeCommand(any(),
+ any());
Object thisResult = executor.execute(parseResult);
assertThat(((CommandResult)
thisResult).getStatus()).isEqualTo(Result.Status.ERROR);
assertThat(thisResult.toString()).contains("my message here");
@@ -107,7 +110,7 @@ public void anyIllegalStateExceptionGetsCaught() throws
Exception {
@Test
public void anyUserErrorExceptionGetsCaught() throws Exception {
;
- doThrow(new UserErrorException("my message
here")).when(executor).invokeCommand(any());
+ doThrow(new UserErrorException("my message
here")).when(executor).invokeCommand(any(), any());
Object thisResult = executor.execute(parseResult);
assertThat(((CommandResult)
thisResult).getStatus()).isEqualTo(Result.Status.ERROR);
assertThat(thisResult.toString()).contains("my message here");
@@ -117,7 +120,7 @@ public void anyUserErrorExceptionGetsCaught() throws
Exception {
public void anyEntityNotFoundException_statusOK() throws Exception {
;
doThrow(new EntityNotFoundException("my message here",
true)).when(executor)
- .invokeCommand(any());
+ .invokeCommand(any(), any());
Object thisResult = executor.execute(parseResult);
assertThat(((CommandResult)
thisResult).getStatus()).isEqualTo(Result.Status.OK);
assertThat(thisResult.toString()).contains("Skipping: my message here");
@@ -126,7 +129,8 @@ public void anyEntityNotFoundException_statusOK() throws
Exception {
@Test
public void anyEntityNotFoundException_statusERROR() throws Exception {
;
- doThrow(new EntityNotFoundException("my message
here")).when(executor).invokeCommand(any());
+ doThrow(new EntityNotFoundException("my message
here")).when(executor).invokeCommand(any(),
+ any());
Object thisResult = executor.execute(parseResult);
assertThat(((CommandResult)
thisResult).getStatus()).isEqualTo(Result.Status.ERROR);
assertThat(thisResult.toString()).contains("my message here");
diff --git
a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
index ff68361aa5..a3c2944898 100644
---
a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
+++
b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
@@ -16,6 +16,7 @@
package org.apache.geode.test.dunit.rules;
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
import static
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.NAME;
import static org.apache.geode.test.dunit.Host.getHost;
@@ -184,6 +185,12 @@ public MemberVM startServerVM(int index, int locatorPort)
throws IOException {
return startServerVM(index, new Properties(), locatorPort);
}
+ public MemberVM startServerVM(int index, String group, int locatorPort)
throws IOException {
+ Properties properties = new Properties();
+ properties.put(GROUPS, group);
+ return startServerVM(index, properties, locatorPort);
+ }
+
public MemberVM startServerVM(int index, Properties properties) throws
IOException {
return startServerVM(index, properties, -1);
}
diff --git
a/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
b/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
index 9ec2754dc2..4b0a43b4e3 100644
---
a/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
+++
b/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
@@ -210,7 +210,8 @@ public CommandResultAssert tableHasRowWithValues(String...
headersThenValues)
}
public CommandResultAssert tableHasRowCount(String anyColumnHeader, int
rowSize) {
-
assertThat(actual.getCommandResult().getColumnValues(anyColumnHeader)).isEqualTo(rowSize);
+
assertThat(actual.getCommandResult().getColumnValues(anyColumnHeader).size())
+ .isEqualTo(rowSize);
return this;
}
diff --git
a/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
b/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
index 20592be04b..a6e181d5db 100644
---
a/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
+++
b/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
@@ -23,7 +23,6 @@
import org.junit.rules.ExternalResource;
import org.springframework.shell.core.Completion;
import org.springframework.shell.core.Converter;
-import org.springframework.util.ReflectionUtils;
import org.apache.geode.internal.ClassPathLoader;
import org.apache.geode.management.cli.CliMetaData;
@@ -32,6 +31,7 @@
import org.apache.geode.management.internal.cli.CommandManager;
import org.apache.geode.management.internal.cli.GfshParseResult;
import org.apache.geode.management.internal.cli.GfshParser;
+import org.apache.geode.management.internal.cli.remote.CommandExecutor;
import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.management.internal.cli.result.ResultBuilder;
import org.apache.geode.test.junit.assertions.CommandResultAssert;
@@ -40,11 +40,13 @@
private GfshParser parser;
private CommandManager commandManager;
+ private CommandExecutor commandExecutor;
@Override
public void before() {
commandManager = new CommandManager();
parser = new GfshParser(commandManager);
+ commandExecutor = new CommandExecutor();
}
public GfshParseResult parse(String command) {
@@ -78,12 +80,13 @@ public GfshParseResult parse(String command) {
}
}
- return (CommandResult)
ReflectionUtils.invokeMethod(parseResult.getMethod(), instance,
- parseResult.getArguments());
+ return (CommandResult) commandExecutor.execute(instance, parseResult);
}
public <T> CommandResultAssert executeAndAssertThat(T instance, String
command) {
CommandResult result = executeCommandWithInstance(instance, command);
+ System.out.println("Command Result:");
+ System.out.println(ResultBuilder.resultAsString(result));
return new CommandResultAssert(result);
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> alter async event queue attributes
> ----------------------------------
>
> Key: GEODE-3788
> URL: https://issues.apache.org/jira/browse/GEODE-3788
> Project: Geode
> Issue Type: Sub-task
> Components: gfsh
> Reporter: Swapnil Bawaskar
>
> We should add a new {{alter async-event-queue}} gfsh command that will allow
> users to change the following attributes on the AsyncEventQueue:
> - batch size
> - batch time interval
> - maximum queue memory
> Attributes changed with this command should only be reflected in cluster
> configuration. We will require users to do a rolling re-start of the servers
> for the new settings to take effect.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
