[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16317707#comment-16317707
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #1253: GEODE-3539: fix test category
URL: https://github.com/apache/geode/pull/1253
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
index 1c61e4a30c..0d924c82f5 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
@@ -26,11 +26,11 @@
import org.apache.geode.internal.cache.wan.MyAsyncEventListener;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
-import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
import org.apache.geode.test.junit.rules.GfshCommandRule;
-@Category(UnitTest.class)
+@Category(DistributedTest.class)
public class ListAsyncEventQueuesCommandDUnitTest {
@ClassRule
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16317708#comment-16317708 ] ASF subversion and git services commented on GEODE-3539: Commit ff008e33a6c3c6a6ab84b96156ec44dad4bc72f7 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=ff008e3 ] GEODE-3539: fix test category (#1253) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16317106#comment-16317106 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1253: GEODE-3539: fix test category URL: https://github.com/apache/geode/pull/1253 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16293446#comment-16293446 ] ASF subversion and git services commented on GEODE-3539: Commit f76eeae1b474cd7a6c86168c8424ab9161e55cee in geode's branch refs/heads/feature/GEODE-3781 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=f76eeae ] GEODE-3539: add tests for ExportStackTraceCommand (#1162) * GEODE-3539: add tests for ExportStackTraceCommand * add unit tests for the command * reworked the DUnit test to use the rules > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16293447#comment-16293447 ] ASF subversion and git services commented on GEODE-3539: Commit f76eeae1b474cd7a6c86168c8424ab9161e55cee in geode's branch refs/heads/feature/GEODE-3781 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=f76eeae ] GEODE-3539: add tests for ExportStackTraceCommand (#1162) * GEODE-3539: add tests for ExportStackTraceCommand * add unit tests for the command * reworked the DUnit test to use the rules > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16292821#comment-16292821 ] ASF subversion and git services commented on GEODE-3539: Commit f76eeae1b474cd7a6c86168c8424ab9161e55cee in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=f76eeae ] GEODE-3539: add tests for ExportStackTraceCommand (#1162) * GEODE-3539: add tests for ExportStackTraceCommand * add unit tests for the command * reworked the DUnit test to use the rules > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16292822#comment-16292822 ] ASF subversion and git services commented on GEODE-3539: Commit f76eeae1b474cd7a6c86168c8424ab9161e55cee in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=f76eeae ] GEODE-3539: add tests for ExportStackTraceCommand (#1162) * GEODE-3539: add tests for ExportStackTraceCommand * add unit tests for the command * reworked the DUnit test to use the rules > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16292820#comment-16292820
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #1162: GEODE-3539: add tests for
ExportStackTraceCommand
URL: https://github.com/apache/geode/pull/1162
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-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportStackTraceCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportStackTraceCommand.java
index 4f8693d382..ddf84dd444 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportStackTraceCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportStackTraceCommand.java
@@ -34,11 +34,9 @@
import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.CliUtil;
import org.apache.geode.management.internal.cli.domain.StackTracesPerMember;
import
org.apache.geode.management.internal.cli.functions.GetStackTracesFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
@@ -51,7 +49,8 @@
private final GetStackTracesFunction getStackTracesFunction = new
GetStackTracesFunction();
/**
- * Current implementation supports writing it to a file and returning the
location of the file
+ * Current implementation supports writing it to a locator/server side file
and returning the
+ * location of the file
*/
@CliCommand(value = CliStrings.EXPORT_STACKTRACE, help =
CliStrings.EXPORT_STACKTRACE__HELP)
@CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DEBUG_UTIL})
@@ -68,55 +67,43 @@ public Result exportStackTrace(@CliOption(key =
{CliStrings.MEMBER, CliStrings.M
help = CliStrings.EXPORT_STACKTRACE__FILE__HELP) String fileName,
@CliOption(key = CliStrings.EXPORT_STACKTRACE__FAIL__IF__FILE__PRESENT,
- unspecifiedDefaultValue = "false",
- help = CliStrings.EXPORT_STACKTRACE__FAIL__IF__FILE__PRESENT__HELP)
boolean failIfFilePresent) {
-
-Result result;
-StringBuilder filePrefix = new StringBuilder("stacktrace");
+ unspecifiedDefaultValue = "false", specifiedDefaultValue = "true",
+ help = CliStrings.EXPORT_STACKTRACE__FAIL__IF__FILE__PRESENT__HELP)
boolean failIfFilePresent)
+ throws IOException {
if (fileName == null) {
+ StringBuilder filePrefix = new StringBuilder("stacktrace");
fileName =
filePrefix.append("_").append(System.currentTimeMillis()).toString();
}
final File outFile = new File(fileName);
-try {
- if (outFile.exists() && failIfFilePresent) {
-return ResultBuilder.createShellClientErrorResult(CliStrings.format(
-CliStrings.EXPORT_STACKTRACE__ERROR__FILE__PRESENT,
outFile.getCanonicalPath()));
- }
-
- InternalCache cache = getCache();
- InternalDistributedSystem ads = cache.getInternalDistributedSystem();
+if (outFile.exists() && failIfFilePresent) {
+ return ResultBuilder.createUserErrorResult(CliStrings
+ .format(CliStrings.EXPORT_STACKTRACE__ERROR__FILE__PRESENT,
outFile.getCanonicalPath()));
+}
- InfoResultData resultData = ResultBuilder.createInfoResultData();
+Map dumps = new HashMap<>();
+Set targetMembers = getMembers(group, memberNameOrId);
- Map dumps = new HashMap<>();
- Set targetMembers = CliUtil.findMembers(group,
memberNameOrId);
- if (targetMembers.isEmpty()) {
-return
ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
- }
+InfoResultData resultData = ResultBuilder.createInfoResultData();
- ResultCollector rc =
- CliUtil.executeFunction(getStackTracesFunction, null, targetMembers);
- ArrayList resultList = (ArrayList) rc.getResult();
+ResultCollector rc = executeFunction(getStackTracesFunction, null,
targetMembers);
+ArrayList resultList = (ArrayList) rc.getResult();
- for (Object resultObj : resultList) {
-if (resultObj instanceof StackTracesPerMember) {
- StackTracesPerMember stackTracePerMember = (StackTracesPerMember)
resultObj;
- dumps.put(stackTracePerMember.getMemberNameOrId(),
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16292730#comment-16292730
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1162: GEODE-3539: add tests
for ExportStackTraceCommand
URL: https://github.com/apache/geode/pull/1162#discussion_r157237492
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStackTraceCommandTest.java
##
@@ -0,0 +1,72 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+
+@Category(UnitTest.class)
+public class ExportStackTraceCommandTest {
+
+ @ClassRule
+ public static GfshParserRule gfsh = new GfshParserRule();
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private ExportStackTraceCommand command;
+
+ @Before
+ public void before() {
+command = spy(ExportStackTraceCommand.class);
+ }
+
+ @Test
+ public void noMemberFound() {
+doReturn(Collections.emptySet()).when(command).findMembers(any(), any());
+gfsh.executeAndAssertThat(command, "export stack-traces").statusIsError()
+.containsOutput("No Members Found");
+ }
+
+ @Test
+ public void fileExists() throws IOException {
Review comment:
fixed now.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16291752#comment-16291752
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1162: GEODE-3539: add
tests for ExportStackTraceCommand
URL: https://github.com/apache/geode/pull/1162#discussion_r157084998
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStackTraceCommandTest.java
##
@@ -0,0 +1,72 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+
+@Category(UnitTest.class)
+public class ExportStackTraceCommandTest {
+
+ @ClassRule
+ public static GfshParserRule gfsh = new GfshParserRule();
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private ExportStackTraceCommand command;
+
+ @Before
+ public void before() {
+command = spy(ExportStackTraceCommand.class);
+ }
+
+ @Test
+ public void noMemberFound() {
+doReturn(Collections.emptySet()).when(command).findMembers(any(), any());
+gfsh.executeAndAssertThat(command, "export stack-traces").statusIsError()
+.containsOutput("No Members Found");
+ }
+
+ @Test
+ public void fileExists() throws IOException {
Review comment:
At first glance, I thought the second assertion was an accidental redundancy
of the test above. I eventually read your comment and realized the test was
verifying that we check for the file existing before anything else.
For clarity, I might prefer a test name like `abortImmediatelyIfFileExists`.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16290080#comment-16290080 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao commented on issue #1162: GEODE-3539: add tests for ExportStackTraceCommand URL: https://github.com/apache/geode/pull/1162#issuecomment-351558488 precheckin green 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16289750#comment-16289750 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1162: GEODE-3539: add tests for ExportStackTraceCommand URL: https://github.com/apache/geode/pull/1162 * add unit tests for the command * reworked the DUnit test to use the rules Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16284259#comment-16284259 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1147: GEODE-3539: more multi-user test coverage. URL: https://github.com/apache/geode/pull/1147 * added more logging in Client's subject binding/removing * added more tests for multi-clients and muti-user-authentication mode. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16275228#comment-16275228 ] ASF subversion and git services commented on GEODE-3539: Commit b2d37ecf15d6e83c238007226ec1d5b170d0fa11 in geode's branch refs/heads/feature/GEODE-3637 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=b2d37ec ] GEODE-3539: enhance GfshCommandRule. Renmame method for consistency. (#) * reduce the default timeout * add ability to configure the timeout at construction time. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16275226#comment-16275226 ] ASF subversion and git services commented on GEODE-3539: Commit c51a455c05d3a94a880569693e8a8e14d176e380 in geode's branch refs/heads/feature/GEODE-3637 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=c51a455 ] GEODE-3539: enhance GfshCommandRule (#1108) * reduce the default timeout * add ability to configure the timeout at construction time. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16274626#comment-16274626
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #: GEODE-3539: enhance GfshCommandRule.
Rename method for consistency.
URL: https://github.com/apache/geode/pull/
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
index 21f3f28ed7..890337b4f2 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
@@ -54,7 +54,7 @@ public static void beforeClass() throws Exception {
}
@ClassRule
- public static GfshCommandRule gfsh = new GfshCommandRule().setTimeout(1);
+ public static GfshCommandRule gfsh = new GfshCommandRule().withTimeout(1);
@Test
public void commandFailsWithoutOptions() throws Exception {
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
index fa438716f0..7961f58a0f 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
@@ -40,7 +40,7 @@
.withName(MEMBER_NAME).withJMXManager().withAutoStart();
@Rule
- public GfshCommandRule gfsh = new GfshCommandRule().setTimeout(1);
+ public GfshCommandRule gfsh = new GfshCommandRule().withTimeout(1);
@Test
public void commandSucceedsWhenConnected() throws Exception {
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListRegionIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListRegionIntegrationTest.java
index 673ebf020c..297893fdd2 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListRegionIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListRegionIntegrationTest.java
@@ -42,11 +42,6 @@
@Rule
public GfshCommandRule gfsh = new GfshCommandRule(server::getJmxPort,
PortType.jmxManager);
- @Before
- public void setup() {
-gfsh.setTimeout(2);
- }
-
@Test
public void commandFailsWhenNotConnected() throws Exception {
gfsh.disconnect();
diff --git
a/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshCommandRule.java
b/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshCommandRule.java
index fb94fa7f2e..6ab7908fe8 100644
---
a/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshCommandRule.java
+++
b/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshCommandRule.java
@@ -268,11 +268,8 @@ public File getWorkingDir() {
return workingDir;
}
- public GfshCommandRule setTimeout(int timeoutInSeconds) {
+ public GfshCommandRule withTimeout(int timeoutInSeconds) {
this.gfshTimeout = timeoutInSeconds;
-if (gfsh != null) {
- gfsh.setTimeout(timeoutInSeconds);
-}
return this;
}
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16274627#comment-16274627 ] ASF subversion and git services commented on GEODE-3539: Commit b2d37ecf15d6e83c238007226ec1d5b170d0fa11 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=b2d37ec ] GEODE-3539: enhance GfshCommandRule. Renmame method for consistency. (#) * reduce the default timeout * add ability to configure the timeout at construction time. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16274556#comment-16274556 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #: GEODE-3539: enhance GfshCommandRule. Rename method for consistency. URL: https://github.com/apache/geode/pull/ * reduce the default timeout * add ability to configure the timeout at construction time. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273976#comment-16273976 ] ASF subversion and git services commented on GEODE-3539: Commit c51a455c05d3a94a880569693e8a8e14d176e380 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=c51a455 ] GEODE-3539: enhance GfshCommandRule (#1108) * reduce the default timeout * add ability to configure the timeout at construction time. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273975#comment-16273975
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #1108: GEODE-3539: enhance GfshCommandRule
URL: https://github.com/apache/geode/pull/1108
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
index 3edc944ce2..21f3f28ed7 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
@@ -18,10 +18,8 @@
import java.util.Arrays;
import java.util.List;
-import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
-import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -56,12 +54,7 @@ public static void beforeClass() throws Exception {
}
@ClassRule
- public static GfshCommandRule gfsh = new GfshCommandRule();
-
- @Before
- public void setTimeout() {
-gfsh.setTimeout(1);
- }
+ public static GfshCommandRule gfsh = new GfshCommandRule().setTimeout(1);
@Test
public void commandFailsWithoutOptions() throws Exception {
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
index 25ac49cdbe..fa438716f0 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListDiskStoreCommandIntegrationTest.java
@@ -15,7 +15,6 @@
package org.apache.geode.management.internal.cli.commands;
-import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
@@ -41,12 +40,7 @@
.withName(MEMBER_NAME).withJMXManager().withAutoStart();
@Rule
- public GfshCommandRule gfsh = new GfshCommandRule();
-
- @Before
- public void setTimeout() {
-gfsh.setTimeout(1);
- }
+ public GfshCommandRule gfsh = new GfshCommandRule().setTimeout(1);
@Test
public void commandSucceedsWhenConnected() throws Exception {
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
index 05b83c96ab..3ad80a91d0 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListMembersCommandDUnitTest.java
@@ -16,6 +16,7 @@
import static org.apache.geode.management.internal.cli.i18n.CliStrings.GROUPS;
import static
org.apache.geode.management.internal.cli.i18n.CliStrings.LIST_MEMBER;
+import static
org.apache.geode.test.junit.rules.GfshCommandRule.PortType.jmxManager;
import static org.assertj.core.api.Assertions.assertThat;
import java.util.Properties;
@@ -38,25 +39,27 @@
private static MemberVM locator;
@Rule
- public GfshCommandRule gfsh = new GfshCommandRule();
+ public GfshCommandRule gfsh = new GfshCommandRule(locator::getJmxPort,
jmxManager);
@BeforeClass
public static void setup() throws Exception {
-locator = lsRule.startLocatorVM(0, propertiesForGroup("locatorGroup"));
-lsRule.startServerVM(1, propertiesForGroup("serverGroup1"),
locator.getPort());
-lsRule.startServerVM(2, propertiesForGroup("serverGroup1"),
locator.getPort());
-lsRule.startServerVM(3, propertiesForGroup("serverGroup2"),
locator.getPort());
+Properties properties = new Properties();
+properties.setProperty(GROUPS, "locatorGroup");
+locator = lsRule.startLocatorVM(0, properties);
+lsRule.startServerVM(1, "serverGroup1", locator.getPort());
+lsRule.startServerVM(2, "serverGroup1", locator.getPort());
+lsRule.startServerVM(3, "serverGroup2", locator.getPort());
}
@Test
public void listMembersWithoutConnection() throws Exception {
+gfsh.disconnect();
gfsh.executeAndAssertThat(LIST_MEMBER).statusIsError()
.containsOutput("Command 'list members' was found but is not currently
available");
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273484#comment-16273484 ] ASF subversion and git services commented on GEODE-3539: Commit de22c2d1b87878fc457e0e24f62011bb5e13a20b in geode's branch refs/heads/feature/GEODE-3923 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=de22c2d ] GEODE-3539: add test coverage for "create async-event-queue" and "lis… (#1093) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273441#comment-16273441 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1108: GEODE-3539: enhance GfshCommandRule URL: https://github.com/apache/geode/pull/1108 * reduce the default timeout * add ability to configure the timeout at construction time. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273094#comment-16273094
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #1104: GEODE-3539: enhance rule to start locator
joining other locators
URL: https://github.com/apache/geode/pull/1104
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
index 4b91ce50c4..92e8c9d8bb 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
@@ -47,7 +47,7 @@
import
org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder;
@Category(DistributedTest.class)
-public class DeployCommandRedeployDUnitTest implements Serializable {
+public class DeployCommandRedeployDUnitTest {
private static final String VERSION1 = "Version1";
private static final String VERSION2 = "Version2";
@@ -173,7 +173,7 @@ private File createJarWithFunctionB(String version) throws
Exception {
return jar;
}
- private void assertThatFunctionHasVersion(String functionId, String version)
{
+ private static void assertThatFunctionHasVersion(String functionId, String
version) {
GemFireCacheImpl gemFireCache = GemFireCacheImpl.getInstance();
DistributedSystem distributedSystem = gemFireCache.getDistributedSystem();
Execution execution =
FunctionService.onMember(distributedSystem.getDistributedMember());
@@ -181,7 +181,8 @@ private void assertThatFunctionHasVersion(String
functionId, String version) {
assertThat(result.get(0)).isEqualTo(version);
}
- private void assertThatCanLoad(String jarName, String className) throws
ClassNotFoundException {
+ private static void assertThatCanLoad(String jarName, String className)
+ throws ClassNotFoundException {
assertThat(ClassPathLoader.getLatest().getJarDeployer().findDeployedJar(jarName)).isNotNull();
assertThat(ClassPathLoader.getLatest().forName(className)).isNotNull();
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
index 0d12a18949..5a1058fb60 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
@@ -15,7 +15,6 @@
package org.apache.geode.management.internal.configuration;
import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
-import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.assertj.core.api.Assertions.assertThat;
@@ -68,14 +67,8 @@ public void testDeployToNoServer() throws Exception {
@Test
public void testDeployToMultipleLocators() throws Exception {
MemberVM locator = lsRule.startLocatorVM(0, locatorProps);
-locatorProps.setProperty(LOCATORS, "localhost[" + locator.getPort() + "]");
-MemberVM locator2 = lsRule.startLocatorVM(1, locatorProps);
-locatorProps.setProperty(LOCATORS,
-"localhost[" + locator.getPort() + "],localhost[" + locator2.getPort()
+ "]");
-MemberVM locator3 = lsRule.startLocatorVM(2, locatorProps);
-
-// has to start a server in order to run deploy command
-lsRule.startServerVM(3, serverProps, locator.getPort());
+MemberVM locator2 = lsRule.startLocatorVM(1, locator.getPort());
+MemberVM locator3 = lsRule.startLocatorVM(2, locator.getPort(),
locator2.getPort());
gfshConnector.connect(locator);
assertThat(gfshConnector.isConnected()).isTrue();
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 c9e2c8337c..1399e2bae8 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
@@ -18,6 +18,7 @@
import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
import static
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273095#comment-16273095 ] ASF subversion and git services commented on GEODE-3539: Commit 375d3684d283c8e3d57174d79a37b3be44b8830c in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=375d368 ] GEODE-3539: enhance rule to start locator joining other locators (#1104) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273033#comment-16273033 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1104: GEODE-3539: enhance rule to start locator joining other locators URL: https://github.com/apache/geode/pull/1104 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16271596#comment-16271596 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao closed pull request #1093: GEODE-3539: add test coverage for "create async-event-queue" and "lis… URL: https://github.com/apache/geode/pull/1093 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-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java index eb89c21b4a..0b46c0b239 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java @@ -23,16 +23,17 @@ import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; -import org.apache.geode.SystemFailure; +import org.apache.geode.cache.asyncqueue.internal.AsyncEventQueueFactoryImpl; +import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.ResultCollector; import org.apache.geode.distributed.DistributedMember; import org.apache.geode.management.cli.ConverterHint; import org.apache.geode.management.cli.Result; -import org.apache.geode.management.internal.cli.CliUtil; import org.apache.geode.management.internal.cli.functions.AsyncEventQueueFunctionArgs; import org.apache.geode.management.internal.cli.functions.CliFunctionResult; import org.apache.geode.management.internal.cli.functions.CreateAsyncEventQueueFunction; import org.apache.geode.management.internal.cli.i18n.CliStrings; +import org.apache.geode.management.internal.cli.result.CommandResult; import org.apache.geode.management.internal.cli.result.ResultBuilder; import org.apache.geode.management.internal.cli.result.TabularResultData; import org.apache.geode.management.internal.configuration.domain.XmlEntity; @@ -52,15 +53,15 @@ public Result createAsyncEventQueue( help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__GROUP__HELP) String[] groups, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__PARALLEL, unspecifiedDefaultValue = "false", specifiedDefaultValue = "true", - help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__PARALLEL__HELP) Boolean parallel, + help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__PARALLEL__HELP) boolean parallel, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__ENABLEBATCHCONFLATION, unspecifiedDefaultValue = "false", specifiedDefaultValue = "true", - help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__ENABLEBATCHCONFLATION__HELP) Boolean enableBatchConflation, + help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__ENABLEBATCHCONFLATION__HELP) boolean enableBatchConflation, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__BATCH_SIZE, unspecifiedDefaultValue = "100", help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__BATCH_SIZE__HELP) int batchSize, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__BATCHTIMEINTERVAL, - unspecifiedDefaultValue = "1000", + unspecifiedDefaultValue = AsyncEventQueueFactoryImpl.DEFAULT_BATCH_TIME_INTERVAL + "", help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__BATCHTIMEINTERVAL__HELP) int batchTimeInterval, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__PERSISTENT, unspecifiedDefaultValue = "false", specifiedDefaultValue = "true", @@ -69,16 +70,16 @@ public Result createAsyncEventQueue( help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__DISK_STORE__HELP) String diskStore, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__DISKSYNCHRONOUS, unspecifiedDefaultValue = "true", specifiedDefaultValue = "true", - help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__DISKSYNCHRONOUS__HELP) Boolean diskSynchronous, + help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__DISKSYNCHRONOUS__HELP) boolean diskSynchronous, @CliOption(key = CliStrings.CREATE_ASYNC_EVENT_QUEUE__FORWARD_EXPIRATION_DESTROY, - unspecifiedDefaultValue = "false", specifiedDefaultValue = "false", - help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__FORWARD_EXPIRATION_DESTROY__HELP) Boolean ignoreEvictionAndExpiration, + unspecifiedDefaultValue = "false", specifiedDefaultValue = "true", + help = CliStrings.CREATE_ASYNC_EVENT_QUEUE__FORWARD_EXPIRATION_DESTROY__HELP) boolean forwardExpirationDestroy, @CliOption(key =
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16271597#comment-16271597 ] ASF subversion and git services commented on GEODE-3539: Commit de22c2d1b87878fc457e0e24f62011bb5e13a20b in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=de22c2d ] GEODE-3539: add test coverage for "create async-event-queue" and "lis… (#1093) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16271061#comment-16271061 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao commented on issue #1093: GEODE-3539: add test coverage for "create async-event-queue" and "lis… URL: https://github.com/apache/geode/pull/1093#issuecomment-347919650 precheckin green 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270163#comment-16270163 ] ASF subversion and git services commented on GEODE-3539: Commit df0a85635deb9efb6e62d00bfce3da5d2348c737 in geode's branch refs/heads/feature/GEODE-4000 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=df0a856 ] GEODE-3539: add ability to stop a vm without clean out the working dir (#1094) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270141#comment-16270141 ] ASF subversion and git services commented on GEODE-3539: Commit df0a85635deb9efb6e62d00bfce3da5d2348c737 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=df0a856 ] GEODE-3539: add ability to stop a vm without clean out the working dir (#1094) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270140#comment-16270140
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #1094: GEODE-3539: add ability to stop a vm
without clean out the working dir
URL: https://github.com/apache/geode/pull/1094
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-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java
index 46d1f7a83e..11795f7a50 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java
@@ -75,8 +75,8 @@ public void exportClusterConfig() throws Exception {
.statusIsSuccess();
gfsh.disconnect();
-locator.stopMember();
-server.stopMember();
+locator.stopMember(true);
+server.stopMember(true);
assertThat(this.exportedClusterConfig).exists();
assertThat(this.exportedClusterConfig.length()).isGreaterThan(100);
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 a3c2944898..c9e2c8337c 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
@@ -301,10 +301,14 @@ public MemberVM startServerAsEmbededLocator(int index,
Properties properties) th
}
public void stopVM(int index) {
+stopVM(index, true);
+ }
+
+ public void stopVM(int index, boolean cleanWorkingDir) {
MemberVM member = members.get(index);
// user has started a server/locator in this VM
if (member != null) {
- member.stopMember();
+ member.stopMember(cleanWorkingDir);
}
// user may have used this VM as a client VM
else {
diff --git
a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java
b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java
index 4e04b8eb8b..a61fabb5e3 100644
--- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java
+++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java
@@ -101,8 +101,12 @@ public int getEmbeddedLocatorPort() {
return ((Server) member).getEmbeddedLocatorPort();
}
- public void stopMember() {
+ public void stopMember(boolean cleanWorkingDir) {
this.invoke(LocatorServerStartupRule::stopMemberInThisVM);
+if (!cleanWorkingDir) {
+ return;
+}
+
if (tempWorkingDir) {
/*
* this temporary workingDir will dynamically change the "user.dir".
system property to point
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269521#comment-16269521
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner commented on a change in pull request #1093: GEODE-3539: add test
coverage for "create async-event-queue" and "lis…
URL: https://github.com/apache/geode/pull/1093#discussion_r153633541
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CreateAsyncEventQueueFunction.java
##
@@ -130,48 +122,19 @@ public void execute(FunctionContext context) {
} catch (CacheClosedException cce) {
context.getResultSender().lastResult(new CliFunctionResult(memberId,
false, null));
-
-} catch (VirtualMachineError e) {
- SystemFailure.initiateFailure(e);
- throw e;
-
-} catch (Throwable th) {
- SystemFailure.checkFailure();
+} catch (Exception th) {
Review comment:
Since the catch is no longer for {{Throwable}} a better name for the
variable would be 'e'
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269516#comment-16269516
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1093: GEODE-3539: add test
coverage for "create async-event-queue" and "lis…
URL: https://github.com/apache/geode/pull/1093#discussion_r153632962
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
##
@@ -0,0 +1,81 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static
org.apache.geode.test.junit.rules.GfshCommandRule.PortType.jmxManager;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.cache.wan.MyAsyncEventListener;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+
+@Category(UnitTest.class)
+public class ListAsyncEventQueuesCommandDUnitTest {
+
+ @ClassRule
+ public static LocatorServerStartupRule lsRule = new
LocatorServerStartupRule();
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule(locator::getJmxPort,
jmxManager);
+
+ private static MemberVM locator;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+locator = lsRule.startLocatorVM(0);
+lsRule.startServerVM(1, "group1", locator.getPort());
+lsRule.startServerVM(2, "group2", locator.getPort());
+ }
+
+ @Test
+ public void list() throws Exception {
+gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+.containsOutput("No Async Event Queues Found");
+
+gfsh.executeAndAssertThat("create async-event-queue --id=queue1
--group=group1 --listener="
++ MyAsyncEventListener.class.getName()).statusIsSuccess();
+
+gfsh.executeAndAssertThat("create async-event-queue --id=queue2
--group=group2 --listener="
++ MyAsyncEventListener.class.getName()).statusIsSuccess();
+
+locator.waitTillAsyncEventQueuesAreReadyOnServers("queue1", 1);
+locator.waitTillAsyncEventQueuesAreReadyOnServers("queue2", 1);
+
+gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+.tableHasRowCount("Member", 2).tableHasRowWithValues("Member", "ID",
"server-1", "queue1")
+.tableHasRowWithValues("Member", "ID", "server-2", "queue2");
+
+// create another async event queue on the entire cluster, verify that the
command will list all
+gfsh.executeAndAssertThat(
+"create async-event-queue --id=queue --listener=" +
MyAsyncEventListener.class.getName())
+.statusIsSuccess();
+
+gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+.tableHasRowCount("Member", 4).tableHasRowWithValues("Member", "ID",
"server-1", "queue1")
+.tableHasRowWithValues("Member", "ID", "server-2", "queue2")
+.tableHasRowWithValues("Member", "ID", "server-1", "queue")
+.tableHasRowWithValues("Member", "ID", "server-2", "queue");
Review comment:
so you are suggesting the caller should call something like:
tableHasRowWithValues("Member,server-2", "ID,queue2")
this would make the processing a bit harder, but still doable. It's just the
way these GFJsonObject are constructed, We will have to refactor out this
entire result object anyway, good things we are lumping all the processing in
this CommandResultAssert object so it's one spot to change.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
>
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269499#comment-16269499
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner commented on a change in pull request #1093: GEODE-3539: add test
coverage for "create async-event-queue" and "lis…
URL: https://github.com/apache/geode/pull/1093#discussion_r153630136
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
##
@@ -0,0 +1,81 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static
org.apache.geode.test.junit.rules.GfshCommandRule.PortType.jmxManager;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.cache.wan.MyAsyncEventListener;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+
+@Category(UnitTest.class)
+public class ListAsyncEventQueuesCommandDUnitTest {
+
+ @ClassRule
+ public static LocatorServerStartupRule lsRule = new
LocatorServerStartupRule();
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule(locator::getJmxPort,
jmxManager);
+
+ private static MemberVM locator;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+locator = lsRule.startLocatorVM(0);
+lsRule.startServerVM(1, "group1", locator.getPort());
+lsRule.startServerVM(2, "group2", locator.getPort());
+ }
+
+ @Test
+ public void list() throws Exception {
+gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+.containsOutput("No Async Event Queues Found");
+
+gfsh.executeAndAssertThat("create async-event-queue --id=queue1
--group=group1 --listener="
++ MyAsyncEventListener.class.getName()).statusIsSuccess();
+
+gfsh.executeAndAssertThat("create async-event-queue --id=queue2
--group=group2 --listener="
++ MyAsyncEventListener.class.getName()).statusIsSuccess();
+
+locator.waitTillAsyncEventQueuesAreReadyOnServers("queue1", 1);
+locator.waitTillAsyncEventQueuesAreReadyOnServers("queue2", 1);
+
+gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+.tableHasRowCount("Member", 2).tableHasRowWithValues("Member", "ID",
"server-1", "queue1")
+.tableHasRowWithValues("Member", "ID", "server-2", "queue2");
+
+// create another async event queue on the entire cluster, verify that the
command will list all
+gfsh.executeAndAssertThat(
+"create async-event-queue --id=queue --listener=" +
MyAsyncEventListener.class.getName())
+.statusIsSuccess();
+
+gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+.tableHasRowCount("Member", 4).tableHasRowWithValues("Member", "ID",
"server-1", "queue1")
+.tableHasRowWithValues("Member", "ID", "server-2", "queue2")
+.tableHasRowWithValues("Member", "ID", "server-1", "queue")
+.tableHasRowWithValues("Member", "ID", "server-2", "queue");
Review comment:
Point of discussion (not necessary to change, but something to consider) :
the signature of {{tableHasRowWithValues}} has {{headersThenValues}}. I didn't
notice this from the previous change to CommandResultAssert, but I think that
that ordering of the {{String}} arguments as {{header, value}} pairs is more
readable. With only one or two values in the row it's not too confusing, but
with a longer list of values of interest, then associating the matching header
and values becomes less clear when reviewing code that calls the method.
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
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269372#comment-16269372 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1094: GEODE-3539: add ability to stop a vm without clean out the working dir URL: https://github.com/apache/geode/pull/1094 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269344#comment-16269344 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1093: GEODE-3539: add test coverage for "create async-event-queue" and "lis… URL: https://github.com/apache/geode/pull/1093 …t async-event-queue" Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267873#comment-16267873 ] ASF subversion and git services commented on GEODE-3539: Commit 40f3ed9dcb9c5f38563356c438a73262ce99d00d in geode's branch refs/heads/feature/GEODE-3781 from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=40f3ed9 ] GEODE-3539: Add test coverage to 'alter disk-store'. * Added AlterDiskStoreJUnitTest for testing of option conflict * Remove subsumed testing from DiskStoreCommandsDUnitTest > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267870#comment-16267870 ] ASF subversion and git services commented on GEODE-3539: Commit f3a021991d239dfc05357d6250d937a90d971f32 in geode's branch refs/heads/feature/GEODE-3781 from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=f3a0219 ] GEODE-3539: Add missing test coverage to 'list disk-stores' and 'describe disk-stores' commands * Refactor cumbersome DUnit test to individual IntegrationTests > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267790#comment-16267790 ] ASF subversion and git services commented on GEODE-3539: Commit 40f3ed9dcb9c5f38563356c438a73262ce99d00d in geode's branch refs/heads/develop from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=40f3ed9 ] GEODE-3539: Add test coverage to 'alter disk-store'. * Added AlterDiskStoreJUnitTest for testing of option conflict * Remove subsumed testing from DiskStoreCommandsDUnitTest > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267789#comment-16267789
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied closed pull request #1070: GEODE-3539: Add test coverage to
'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreDUnitTest.java
new file mode 100644
index 00..566e88b616
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreDUnitTest.java
@@ -0,0 +1,168 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+
+@Category(DistributedTest.class)
+public class AlterDiskStoreDUnitTest {
+ private static final String regionName = "region1";
+ private static final String diskStoreName = "disk-store1";
+ private static final String diskDirName = "diskStoreDir";
+ private static final String aKey = "key1";
+
+ private static MemberVM locator;
+ private static MemberVM server1;
+
+ @Rule
+ public LocatorServerStartupRule startupRule = new LocatorServerStartupRule();
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Before
+ public void setupCluster() throws Exception {
+
+File diskStoreDirFile = new File(getDiskDirPathString());
+
+if (!diskStoreDirFile.exists()) {
+ diskStoreDirFile.mkdirs();
+}
+
+locator = startupRule.startLocatorVM(0);
+server1 = startupRule.startServerVM(1, locator.getPort());
+String diskDirPathString = getDiskDirPathString();
+server1.invoke(() -> {
+ Cache cache = LocatorServerStartupRule.getCache();
+ cache.createDiskStoreFactory().setDiskDirs(new File[] {new
File(diskDirPathString)})
+
.setMaxOplogSize(1).setAllowForceCompaction(true).setAutoCompact(false)
+ .create(diskStoreName);
+
+ EvictionAttributes ea =
+ EvictionAttributes.createLRUEntryAttributes(1,
EvictionAction.OVERFLOW_TO_DISK);
+
+ RegionFactory regionFactory =
cache.createRegionFactory();
+ Region region =
regionFactory.setDiskStoreName(diskStoreName)
+
.setDiskSynchronous(true).setDataPolicy(DataPolicy.PERSISTENT_REPLICATE)
+
.setScope(Scope.DISTRIBUTED_ACK).setEvictionAttributes(ea).create(regionName);
+ region.put(aKey, "value");
+});
+ }
+
+ @Test
+ public void diskStoreIsLockedWhileMemberIsAlive() throws Exception {
+gfsh.connectAndVerify(locator.getJmxPort(), PortType.jmxManager);
+String commandString = commandToSetManyVariables();
+
gfsh.executeAndAssertThat(commandString).statusIsError().containsOutput("Could
not lock",
+
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267129#comment-16267129
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r153272359
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreIntegrationTest.java
##
@@ -0,0 +1,60 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.io.IOException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class AlterDiskStoreIntegrationTest {
Review comment:
this can be done in a unit test using GfshParserRule
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263475#comment-16263475
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152693598
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreDUnitTest.java
##
@@ -0,0 +1,170 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+
+@Category(DistributedTest.class)
+public class AlterDiskStoreDUnitTest implements Serializable {
+ private static final String regionName = "region1";
+ private static final String diskStoreName = "disk-store1";
+ private static final String diskDirName = "diskStoreDir";
+ private static final String aKey = "key1";
+
+ private static MemberVM locator;
+ private static MemberVM server1;
+
+ @Rule
+ public LocatorServerStartupRule startupRule = new
LocatorServerStartupRule().withTempWorkingDir();
Review comment:
I was a bit worried that just using the `gfsh` working directory could cause
problems if the `--remove` test was run first, but that appears to have been a
misplaced fear.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263462#comment-16263462
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152691736
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreIntegrationTest.java
##
@@ -0,0 +1,67 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.io.IOException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class AlterDiskStoreIntegrationTest {
+ private static final String regionName = "region1";
+ private static final String memberName = "server";
+ private static final String diskStoreName = "disk-store1";
+
+ @Rule
+ public ServerStarterRule server =
Review comment:
Can the GfshParserRule do that? I thought that rule only determined if the
string could find the function to call, not to test the mutual exclusion of
options within the code itself.
Definitely don't need the server, though. I missed that when I pulled this
out of the DUnit.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263456#comment-16263456
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152691736
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreIntegrationTest.java
##
@@ -0,0 +1,67 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.io.IOException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class AlterDiskStoreIntegrationTest {
+ private static final String regionName = "region1";
+ private static final String memberName = "server";
+ private static final String diskStoreName = "disk-store1";
+
+ @Rule
+ public ServerStarterRule server =
Review comment:
~~Can the GfshParserRule do that? I thought that rule only determined if
the string could find the function to call, not to test the mutual exclusion of
options within the code itself.~~ Oh, I was only looking at `.parse` and not
`executeCommandWithInstance`. Gotcha.
Definitely don't need the server, though. I missed that when I pulled this
out of the DUnit.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263454#comment-16263454
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152691736
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreIntegrationTest.java
##
@@ -0,0 +1,67 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.io.IOException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class AlterDiskStoreIntegrationTest {
+ private static final String regionName = "region1";
+ private static final String memberName = "server";
+ private static final String diskStoreName = "disk-store1";
+
+ @Rule
+ public ServerStarterRule server =
Review comment:
Can the GfshParserRule do that? I thought that rule only determined if the
string could find the function to call, not to test the mutual exclusion of
options within the code itself.
Definitely don't need the server, though. I missed that when I pulled this
out of the DUnit.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263403#comment-16263403
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152685714
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreDUnitTest.java
##
@@ -0,0 +1,170 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+
+@Category(DistributedTest.class)
+public class AlterDiskStoreDUnitTest implements Serializable {
Review comment:
I was running into some RMIs during testing and had thought that was the
culprit, looking at other DUnits. But it looks like I was wrong on that front,
or the root cause was massaged out. Will remove.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263397#comment-16263397
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied closed pull request #1065: GEODE-3539: Add missing test coverage
to 'list disk-stores' and …
URL: https://github.com/apache/geode/pull/1065
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
new file mode 100644
index 00..3edc944ce2
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
@@ -0,0 +1,108 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class DescribeDiskStoreCommandIntegrationTest {
+ private static final String REGION_NAME = "test-region";
+ private static final String MEMBER_NAME = "testServer";
+ private static final String DISK_STORE_NAME = "testDiskStore";
+
+ private static final List expectedData = Arrays.asList("Disk Store
ID", "Disk Store Name",
+ "Member ID", "Member Name", "Allow Force Compaction", "Auto Compaction",
+ "Compaction Threshold", "Max Oplog Size", "Queue Size", "Time Interval",
"Write Buffer Size",
+ "Disk Usage Warning Percentage", "Disk Usage Critical Percentage ",
+ "PDX Serialization Meta-Data Stored", "Disk Directory", "Size");
+
+ @ClassRule
+ public static ServerStarterRule server =
+ new ServerStarterRule().withRegion(RegionShortcut.REPLICATE, REGION_NAME)
+ .withName(MEMBER_NAME).withJMXManager().withAutoStart();
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+server.getCache().createDiskStoreFactory().create(DISK_STORE_NAME);
+gfsh.connectAndVerify(server.getJmxPort(), PortType.jmxManager);
+
+ }
+
+ @ClassRule
+ public static GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Before
+ public void setTimeout() {
+gfsh.setTimeout(1);
+ }
+
+ @Test
+ public void commandFailsWithoutOptions() throws Exception {
+String cmd = "describe disk-store";
+gfsh.executeAndAssertThat(cmd).statusIsError().containsOutput("You should
specify option (",
+"--name", "--member", ") for this command");
+
+ }
+
+ @Test
+ public void commandFailsWithOnlyMember() throws Exception {
+String cmd = "describe disk-store --member=" + MEMBER_NAME;
+gfsh.executeAndAssertThat(cmd).statusIsError().containsOutput("You should
specify option (",
+"--name", ") for this command");
+ }
+
+ @Test
+ public void commandFailsWithOnlyName() throws Exception {
+String cmd = "describe disk-store --name=" + DISK_STORE_NAME;
+gfsh.executeAndAssertThat(cmd).statusIsError().containsOutput("You should
specify option (",
+"--member", ") for this command");
+ }
+
+ @Test
+ public void commandFailsWithBadMember() throws Exception {
+String cmd = "describe disk-store --member=invalid-member-name --name=" +
DISK_STORE_NAME;
+gfsh.executeAndAssertThat(cmd).statusIsError().containsOutput("Member",
+"could not be found. Please verify the member name or
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263398#comment-16263398 ] ASF subversion and git services commented on GEODE-3539: Commit f3a021991d239dfc05357d6250d937a90d971f32 in geode's branch refs/heads/develop from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=f3a0219 ] GEODE-3539: Add missing test coverage to 'list disk-stores' and 'describe disk-stores' commands * Refactor cumbersome DUnit test to individual IntegrationTests > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260783#comment-16260783 ] ASF subversion and git services commented on GEODE-3539: Commit 3a63e609136de4196e4ae4d808358b8dfb8b3116 in geode's branch refs/heads/develop from [~jens.deppe] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=3a63e60 ] GEODE-2567: Update AnalyzeSerializables - This also includes a change for GEODE-3539 > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260325#comment-16260325
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152189334
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreDUnitTest.java
##
@@ -0,0 +1,170 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+
+@Category(DistributedTest.class)
+public class AlterDiskStoreDUnitTest implements Serializable {
+ private static final String regionName = "region1";
+ private static final String diskStoreName = "disk-store1";
+ private static final String diskDirName = "diskStoreDir";
+ private static final String aKey = "key1";
+
+ private static MemberVM locator;
+ private static MemberVM server1;
+
+ @Rule
+ public LocatorServerStartupRule startupRule = new
LocatorServerStartupRule().withTempWorkingDir();
Review comment:
withTempWorkingDir will cause VM to bounce after tests, do we really need it
to be in tempWorkignDir. A regular rule will have a workingDir setup as well.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260326#comment-16260326
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1070: GEODE-3539: Add test
coverage to 'alter disk-store'.
URL: https://github.com/apache/geode/pull/1070#discussion_r152188911
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreIntegrationTest.java
##
@@ -0,0 +1,67 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.io.IOException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class AlterDiskStoreIntegrationTest {
+ private static final String regionName = "region1";
+ private static final String memberName = "server";
+ private static final String diskStoreName = "disk-store1";
+
+ @Rule
+ public ServerStarterRule server =
Review comment:
this is an offline command, we don't really need a running locator/server to
test it.
Use GfshParserRule to test this validation.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16259519#comment-16259519 ] ASF subversion and git services commented on GEODE-3539: Commit 526bcfc073d5cdc448d072269717743292248402 in geode's branch refs/heads/develop from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=526bcfc ] GEODE-3539: Add missing test coverage for 'list regions' and 'describe region' commands * Command logic flattened to stream. * Testing of 'list region' and 'describe region' separated into distinct DUnit and Integration tests. 'list region' DUnit simplified. * Removed @Flaky annotation from 'describe region' DUnit test > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16259518#comment-16259518
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied closed pull request #1052: GEODE-3539: Add missing test coverage
for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052
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-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
index 1ca310cc6f..cd58449cc2 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
@@ -16,9 +16,12 @@
package org.apache.geode.management.internal.cli.commands;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.LinkedHashSet;
+import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
+import java.util.stream.Collectors;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
@@ -29,7 +32,9 @@
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.GfshParseResult;
import org.apache.geode.management.internal.cli.domain.RegionInformation;
import org.apache.geode.management.internal.cli.functions.GetRegionsFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
@@ -67,19 +72,11 @@ public Result listRegion(
ArrayList resultList = (ArrayList) rc.getResult();
if (resultList != null) {
-
-for (Object resultObj : resultList) {
- if (resultObj != null) {
-if (resultObj instanceof Object[]) {
- Object[] resultObjectArray = (Object[]) resultObj;
- for (Object regionInfo : resultObjectArray) {
-if (regionInfo instanceof RegionInformation) {
- regionInfoSet.add((RegionInformation) regionInfo);
-}
- }
-}
- }
-}
+// Gather all RegionInformation into a flat set.
+regionInfoSet.addAll(resultList.stream().filter(Objects::nonNull)
+
.filter(Object[].class::isInstance).map(Object[].class::cast).flatMap(Arrays::stream)
+
.filter(RegionInformation.class::isInstance).map(RegionInformation.class::cast)
+.collect(Collectors.toSet()));
Set regionNames = new TreeSet<>();
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/HeadlessGfsh.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/HeadlessGfsh.java
index 340712ca4f..97f600b658 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/HeadlessGfsh.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/HeadlessGfsh.java
@@ -121,7 +121,7 @@ public void handleExecutionResult(Object result, String
sysout) {
}
public Object getResult() throws InterruptedException {
-// Dont wait for when some command calls gfsh.stop();
+// Don't wait for when some command calls gfsh.stop();
if (shell.stopCalledThroughAPI)
return null;
try {
@@ -146,6 +146,10 @@ public void clear() {
outputString = null;
}
+ public void setTimeout(long timeout) {
+this.timeout = timeout;
+ }
+
public void clearEvents() {
queue.clear();
outputString = null;
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAndDescribeRegionDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeRegionDUnitTest.java
similarity index 78%
rename from
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAndDescribeRegionDUnitTest.java
rename to
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeRegionDUnitTest.java
index f85816e645..8e223394cd 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAndDescribeRegionDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeRegionDUnitTest.java
@@ -59,7 +59,7 @@
import org.apache.geode.test.junit.rules.GfshCommandRule;
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16257794#comment-16257794 ] ASF subversion and git services commented on GEODE-3539: Commit cd0b65aa0d476037000b3f24da359e0111529fe4 in geode's branch refs/heads/develop from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=cd0b65a ] GEODE-3539: Restore and correct test coverage for 'describe connection' command. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16257793#comment-16257793
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied closed pull request #1024: GEODE-3539: Restore test coverage for
'describe connection' command.
URL: https://github.com/apache/geode/pull/1024
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
new file mode 100644
index 00..7db73af5c9
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
@@ -0,0 +1,65 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import org.apache.logging.log4j.Logger;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+/**
+ * The GfshCommandJUnitTest class is a test suite of test cases testing the
contract and
+ * functionality of the GfshCommand class for implementing GemFire shell
(Gfsh) commands.
+ *
+ * @see org.apache.geode.management.internal.cli.commands.GfshCommand
+ * @see org.jmock.Expectations
+ * @see org.jmock.Mockery
+ * @see org.jmock.lib.legacy.ClassImposteriser
+ * @see org.junit.Assert
+ * @see org.junit.Test
+ * @since GemFire 7.0
+ */
+
+@Category(IntegrationTest.class)
+public class DescribeConnectionCommandJUnitTest {
+ public static Logger logger = LogService.getLogger();
+
+ @ClassRule
+ public static LocatorStarterRule locator = new
LocatorStarterRule().withAutoStart();
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Test
+ public void describeConnectionTest() throws Exception {
+gfsh.connectAndVerify(locator.getJmxPort(), PortType.jmxManager);
+gfsh.executeAndAssertThat("describe
connection").tableHasColumnWithValuesContaining(
+"Connection Endpoints",
gfsh.getGfsh().getOperationInvoker().toString());
+ }
+
+ @Test
+ public void executeWhileNotConnected() throws Exception {
+gfsh.executeAndAssertThat("describe connection")
+.tableHasColumnWithValuesContaining("Connection Endpoints", "Not
connected");
+ }
+
+}
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16257791#comment-16257791 ] ASF GitHub Bot commented on GEODE-3539: --- PurelyApplied commented on issue #1052: GEODE-3539: Add missing test coverage for 'list regions' and 'describe region' commands URL: https://github.com/apache/geode/pull/1052#issuecomment-345399727 Rebased and updated per requests, precheckin running. Changes to `GfshCommandRule` and `HeadlessGfsh` copied directly from PR #1065. Verify that no strange conflict exists before rebasing / merging. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16257787#comment-16257787 ] ASF GitHub Bot commented on GEODE-3539: --- PurelyApplied commented on issue #1065: GEODE-3539: Add missing test coverage to 'list disk-stores' and … URL: https://github.com/apache/geode/pull/1065#issuecomment-345398498 Rebased and updated per requests, precheckin running. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16257523#comment-16257523 ] ASF GitHub Bot commented on GEODE-3539: --- PurelyApplied opened a new pull request #1070: GEODE-3539: Add test coverage to 'alter disk-store'. URL: https://github.com/apache/geode/pull/1070 * Remove flaky testing from DiskStoreCommandsDUnitTest, converted to AlterDiskStoreDUnitTest * Added AlterDiskStoreIntegrationTest for testing of option conflict --- Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16255662#comment-16255662
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1065: GEODE-3539: Add missing
test coverage to 'list disk-stores' and …
URL: https://github.com/apache/geode/pull/1065#discussion_r151484942
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
##
@@ -0,0 +1,112 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class DescribeDiskStoreCommandIntegrationTest {
+ private static final String REGION_NAME = "test-region";
+ private static final String MEMBER_NAME = "testServer";
+ private static final String DISK_STORE_NAME = "testDiskStore";
+
+ private static final List expectedData = Arrays.asList("Disk Store
ID", "Disk Store Name",
+ "Member ID", "Member Name", "Allow Force Compaction", "Auto Compaction",
+ "Compaction Threshold", "Max Oplog Size", "Queue Size", "Time Interval",
"Write Buffer Size",
+ "Disk Usage Warning Percentage", "Disk Usage Critical Percentage ",
+ "PDX Serialization Meta-Data Stored", "Disk Directory", "Size");
+
+ @ClassRule
+ public static ServerStarterRule server =
+ new ServerStarterRule().withRegion(RegionShortcut.REPLICATE, REGION_NAME)
+
.withName(MEMBER_NAME).withJMXManager().withEmbeddedLocator().withAutoStart();
+
+ @BeforeClass
+ public static void beforeClass() {
+server.getCache().createDiskStoreFactory().create(DISK_STORE_NAME);
+ }
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Before
+ public void setTimeout() {
+gfsh.setTimeout(1);
+ }
+
+ @Test
+ public void commandFailsWithoutOptions() throws Exception {
+String cmd = "describe disk-store";
+gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
Review comment:
you can put your connectAndVerify in the beforeClass() method
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254565#comment-16254565
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1052: GEODE-3539: Add missing
test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151294390
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
##
@@ -110,4 +108,17 @@ public Result listRegion(
}
return result;
}
+
+ public static class ListRegionInterceptor extends
AbstractCliAroundInterceptor {
Review comment:
You probably did not rebase to the current develop, I just made a change in
CommandExecutor to process UserErrorException differently than general
exception. That would probably ease your concern.
The getMemberXXX call would have to throw an exception anyway to catch the
cases where some commands do not do that validation in their interceptors. So
that pair of throw/catch needs to exist anyway. But it's your call whether you
add an interceptor to handle it for this command or not.
You are right Exceptions should be exceptional. We are trying to move the
commands away from using getXXX calls (which throws an exception when not
found) and uses findXXX which would either return null or empty list, and your
code would handle what to return when nothing is found. But that's for another
ticket.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254552#comment-16254552
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1052: GEODE-3539: Add missing
test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151294390
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
##
@@ -110,4 +108,17 @@ public Result listRegion(
}
return result;
}
+
+ public static class ListRegionInterceptor extends
AbstractCliAroundInterceptor {
Review comment:
You probably did not rebase to the current develop, I just made a change in
CommandExecutor to process UserErrorException differently than general
exception. That would probably ease your concern.
The getMemberXXX call would have to throw an exception anyway to catch the
cases where some commands do not do that validation in their interceptors. So
that pair of throw/catch needs to exist anyway. But it's your call whether you
add an interceptor to handle it for this command or not.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254550#comment-16254550
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1052: GEODE-3539: Add missing
test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151294390
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
##
@@ -110,4 +108,17 @@ public Result listRegion(
}
return result;
}
+
+ public static class ListRegionInterceptor extends
AbstractCliAroundInterceptor {
Review comment:
You probably did not rebase to the current develop, I just made a change in
CommandExecutor to process UserErrorException differently than general
exception. That would probably ease your concern.
The getMemberXXX call would have to throw an exception anyway to catch the
cases where some commands do not do that validation in their interceptors.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254504#comment-16254504 ] ASF GitHub Bot commented on GEODE-3539: --- PurelyApplied opened a new pull request #1065: GEODE-3539: Add missing test coverage to 'list disk-stores' and … URL: https://github.com/apache/geode/pull/1065 'describe disk-store' commands * Refactor cumbersome DUnit test to individual IntegrationTests * Added setTimeout to GfshCommandRule / HeadlessGfsh for quicker failure in bad parameter tests. --- Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254280#comment-16254280
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1052: GEODE-3539: Add
missing test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151263537
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
##
@@ -110,4 +108,17 @@ public Result listRegion(
}
return result;
}
+
+ public static class ListRegionInterceptor extends
AbstractCliAroundInterceptor {
Review comment:
I think cleaner is better, too, but throwing inside the command server-side
feels a little too close to the "exception as program flow" antipattern.
Exceptions should be exceptional, right?
There's also the issue that such an exception then gets wrapped a couple of
times, to result in the output
```
Could not process command due to error. Error occurred while fetching list
of regions. : Please provide either "member" or "group" option.
```
This could be improved by a better message in the root exception, but still
would have two confusing layers of output for something that is ultimately a
User error, not a server-error.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254085#comment-16254085
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1052: GEODE-3539: Add missing
test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151238253
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeRegionIntegrationTest.java
##
@@ -0,0 +1,70 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class DescribeRegionIntegrationTest {
+ private static String MEMBER_NAME = "test-server";
+ private static String REGION_NAME = "test-region";
+ private static String GROUP_NAME = "test-group";
+
+ @ClassRule
+ public static ServerStarterRule server = new ServerStarterRule()
+ .withRegion(RegionShortcut.REPLICATE, REGION_NAME).withName(MEMBER_NAME)
+ .withProperty("groups",
GROUP_NAME).withJMXManager().withEmbeddedLocator().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void commandFailsWhenNotConnected() throws Exception {
+gfsh.executeAndAssertThat("describe region").statusIsError()
+.containsOutput("was found but is not currently available");
+ }
+
+ @Test
+ public void commandFailsWithoutNameOption() throws Exception {
Review comment:
yeah, that would be a good solution too.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16253898#comment-16253898
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1052: GEODE-3539: Add
missing test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151210741
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeRegionIntegrationTest.java
##
@@ -0,0 +1,70 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class DescribeRegionIntegrationTest {
+ private static String MEMBER_NAME = "test-server";
+ private static String REGION_NAME = "test-region";
+ private static String GROUP_NAME = "test-group";
+
+ @ClassRule
+ public static ServerStarterRule server = new ServerStarterRule()
+ .withRegion(RegionShortcut.REPLICATE, REGION_NAME).withName(MEMBER_NAME)
+ .withProperty("groups",
GROUP_NAME).withJMXManager().withEmbeddedLocator().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void commandFailsWhenNotConnected() throws Exception {
+gfsh.executeAndAssertThat("describe region").statusIsError()
+.containsOutput("was found but is not currently available");
+ }
+
+ @Test
+ public void commandFailsWithoutNameOption() throws Exception {
Review comment:
Would that be preferable to just lowering the timeout on `HeadlessGfsh` for
commands that should be executed quickly? I see that the timeout is otherwise
only set at instantiation of the `HeadlessGfsh` instance.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16253700#comment-16253700 ] ASF subversion and git services commented on GEODE-3539: Commit 740ce79f025478d8773f3b14c60755621d10c551 in geode's branch refs/heads/develop from [~khowe] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=740ce79 ] GEODE-3539: Correct typo in javadoc annotation (#1057) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16253699#comment-16253699
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner closed pull request #1057: GEODE-3539: Correct typo in javadoc
annotation
URL: https://github.com/apache/geode/pull/1057
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-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
index 7e4ae044df..02eb8b81a6 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
@@ -597,7 +597,7 @@ public static boolean contains(Object[] array, Object
object) {
*
* @param regions
* @param cache
- * @param returnAll: if true, returns all matching members, otherwise,
returns only one.
+ * @param returnAll if true, returns all matching members, otherwise,
returns only one.
*/
public static Set
getQueryRegionsAssociatedMembers(Set regions,
final InternalCache cache, boolean returnAll) {
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16252244#comment-16252244 ] ASF subversion and git services commented on GEODE-3539: Commit 576bf716cbe75cff49e492bf800d84f525321b38 in geode's branch refs/heads/feature/GEODE-3940 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=576bf71 ] GEODE-3539: rename GfshShellConnectionRule to GfshCommandRule (#1051) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16252079#comment-16252079 ] ASF GitHub Bot commented on GEODE-3539: --- pdxrunner opened a new pull request #1057: GEODE-3539: Correct typo in javadoc annotation URL: https://github.com/apache/geode/pull/1057 This cleans up a build warning. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [X] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [X] Is your initial contribution a single, squashed commit? - [X] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16251671#comment-16251671
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1052: GEODE-3539: Add missing
test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r150887072
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeRegionIntegrationTest.java
##
@@ -0,0 +1,70 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule.PortType;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(IntegrationTest.class)
+public class DescribeRegionIntegrationTest {
+ private static String MEMBER_NAME = "test-server";
+ private static String REGION_NAME = "test-region";
+ private static String GROUP_NAME = "test-group";
+
+ @ClassRule
+ public static ServerStarterRule server = new ServerStarterRule()
+ .withRegion(RegionShortcut.REPLICATE, REGION_NAME).withName(MEMBER_NAME)
+ .withProperty("groups",
GROUP_NAME).withJMXManager().withEmbeddedLocator().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void commandFailsWhenNotConnected() throws Exception {
+gfsh.executeAndAssertThat("describe region").statusIsError()
+.containsOutput("was found but is not currently available");
+ }
+
+ @Test
+ public void commandFailsWithoutNameOption() throws Exception {
Review comment:
just a minor point: invalid command that failed parser takes a long time to
run in the dunit/integration test. Better put them in a unit test and uses
GfshParserRule to test them.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16251670#comment-16251670
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1052: GEODE-3539: Add missing
test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r150885659
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
##
@@ -110,4 +108,17 @@ public Result listRegion(
}
return result;
}
+
+ public static class ListRegionInterceptor extends
AbstractCliAroundInterceptor {
Review comment:
the call of getMember or findMember inside the command will throw an
exception if both are set, so we don't really need to do the check here. I
admit that this brings the validation to the client side, but it's a bit of a
redundant code. I think we can afford to make that check on the server side if
it makes the code a little cleaner.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16251592#comment-16251592
]
ASF GitHub Bot commented on GEODE-3539:
---
jdeppe-pivotal commented on a change in pull request #1052: GEODE-3539: Add
missing test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r150873388
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListRegionDUnitTest.java
##
@@ -0,0 +1,199 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static
org.apache.geode.distributed.ConfigurationProperties.ENABLE_TIME_STATISTICS;
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static
org.apache.geode.distributed.ConfigurationProperties.STATISTIC_SAMPLING_ENABLED;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.GROUP;
+import static
org.apache.geode.management.internal.cli.i18n.CliStrings.LIST_REGION;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.MEMBER;
+
+import java.util.Properties;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.FixedPartitionAttributes;
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+
+@Category(DistributedTest.class)
+public class ListRegionDUnitTest {
+ private static final String REGION1 = "region1";
+ private static final String REGION2 = "region2";
+ private static final String REGION3 = "region3";
+ private static final String SUBREGION1A = "subregion1A";
+ private static final String SUBREGION1B = "subregion1B";
+ private static final String SUBREGION1C = "subregion1C";
+ private static final String PR1 = "PR1";
+ private static final String LOCALREGIONONMANAGER = "LocalRegionOnManager";
+
+ @ClassRule
+ public static LocatorServerStartupRule lsRule = new
LocatorServerStartupRule();
+
+ @ClassRule
+ public static GfshShellConnectionRule gfshShellConnectionRule = new
GfshShellConnectionRule();
+
+ @BeforeClass
+ public static void setupSystem() throws Exception {
+final Properties locatorProps = createProperties("Locator", "G3");
+MemberVM locator = lsRule.startLocatorVM(0, locatorProps);
+
+final Properties managerProps = createProperties("Manager", "G1");
+managerProps.setProperty(LOCATORS, "localhost[" + locator.getPort() + "]");
+MemberVM manager = lsRule.startServerVM(1, managerProps,
locator.getPort());
Review comment:
This isn't really a manager, is it?
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
>
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16251591#comment-16251591
]
ASF GitHub Bot commented on GEODE-3539:
---
jdeppe-pivotal commented on a change in pull request #1052: GEODE-3539: Add
missing test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r150873828
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListRegionDUnitTest.java
##
@@ -0,0 +1,199 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import static
org.apache.geode.distributed.ConfigurationProperties.ENABLE_TIME_STATISTICS;
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static
org.apache.geode.distributed.ConfigurationProperties.STATISTIC_SAMPLING_ENABLED;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.GROUP;
+import static
org.apache.geode.management.internal.cli.i18n.CliStrings.LIST_REGION;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.MEMBER;
+
+import java.util.Properties;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.FixedPartitionAttributes;
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+
+@Category(DistributedTest.class)
+public class ListRegionDUnitTest {
+ private static final String REGION1 = "region1";
+ private static final String REGION2 = "region2";
+ private static final String REGION3 = "region3";
+ private static final String SUBREGION1A = "subregion1A";
+ private static final String SUBREGION1B = "subregion1B";
+ private static final String SUBREGION1C = "subregion1C";
+ private static final String PR1 = "PR1";
+ private static final String LOCALREGIONONMANAGER = "LocalRegionOnManager";
+
+ @ClassRule
+ public static LocatorServerStartupRule lsRule = new
LocatorServerStartupRule();
+
+ @ClassRule
+ public static GfshShellConnectionRule gfshShellConnectionRule = new
GfshShellConnectionRule();
+
+ @BeforeClass
+ public static void setupSystem() throws Exception {
Review comment:
There's a ton of setup in here that doesn't seem to be relevant to what is
actually being tested. Can this be trimmed down to the minimum to get the tests
to pass?
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16250837#comment-16250837 ] ASF subversion and git services commented on GEODE-3539: Commit 576bf716cbe75cff49e492bf800d84f525321b38 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=576bf71 ] GEODE-3539: rename GfshShellConnectionRule to GfshCommandRule (#1051) > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16250474#comment-16250474 ] ASF GitHub Bot commented on GEODE-3539: --- PurelyApplied opened a new pull request #1052: GEODE-3539: Add missing test coverage for 'list regions' and 'describe region' commands URL: https://github.com/apache/geode/pull/1052 * Mutually exclusive options --group and --member checked in interceptor instead of failing server-side * Command logic flattened to stream. * Testing of 'list region' and 'describe region' separated into distinct DUnit and Integration tests * Removed @Flaky annotation from 'describe region' DUnit test. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16250247#comment-16250247 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1051: GEODE-3539: rename GfshShellConnectionRule to GfshCommandRule URL: https://github.com/apache/geode/pull/1051 rename only 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16248330#comment-16248330 ] ASF subversion and git services commented on GEODE-3539: Commit e67456dbce7d2781ad40824cb9e20b67bf7c4074 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=e67456d ] GEODE-3539: cleanup GfshCommand and refactor tests * remove more try catch blocks in commands * make GfshCommand a pure wrapper around CliUtil for easy mocking * unify EntityNotFoundException > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16247924#comment-16247924 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1045: GEODE-3539: cleanup GfshCommand and refactor tests URL: https://github.com/apache/geode/pull/1045 * remove more try catch blocks in commands * make GfshCommand a pure wrapper around CliUtil for easy mocking * Generalize MemberNotFoundException and DiskStoreNotFoundException into EntityNotFoundException Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16247027#comment-16247027 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao commented on issue #1040: GEODE-3539: cleanup GfshCommand and refactor tests URL: https://github.com/apache/geode/pull/1040#issuecomment-343377984 precheckin green 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16246712#comment-16246712
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner commented on a change in pull request #1040: GEODE-3539: cleanup
GfshCommand and refactor tests
URL: https://github.com/apache/geode/pull/1040#discussion_r150106964
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
##
@@ -73,4 +83,35 @@ public void whenNoRegionIsFoundOnAnyMembers() throws
Exception {
result = parser.executeCommandWithInstance(command, "destroy region
--name=test --if-exists");
assertThat(result.getStatus()).isEqualTo(Result.Status.OK);
}
+
+ @Test
+ public void multipleResultReturnedWithOneError() throws Exception {
+// mock this to pass the member search call
+doReturn(Collections.singleton(DistributedMember.class)).when(command)
+.findMembersForRegion(any(), any());
+
+ResultCollector collector = mock(ResultCollector.class);
+doReturn(collector).when(command).executeFunction(any(), any(),
any(Set.class));
+
+List functionResults = new ArrayList<>();
+doReturn(functionResults).when(collector).getResult();
+CliFunctionResult result1 = mock(CliFunctionResult.class);
+CliFunctionResult result2 = mock(CliFunctionResult.class);
+functionResults.add(result1);
+functionResults.add(result2);
+
+when(result1.isSuccessful()).thenReturn(true);
+when(result1.getMessage()).thenReturn("result1 message");
+when(result1.getXmlEntity()).thenReturn(mock(XmlEntity.class));
+
+when(result2.isSuccessful()).thenReturn(false);
+when(result2.getMessage()).thenReturn("result2 message");
+
+result = parser.executeCommandWithInstance(command, "destroy region
--name=test");
+assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
+assertThat(result.getContent().toString()).contains("result2 message");
+
+// verify that xmlEntiry returned by the result1 is not saved to Cluster
config
+verify(command, never()).persistClusterConfiguration(any(), any());
+ }
Review comment:
Could have a couple more tests, this one multiple-results tests looks like
it misses coverage of some conditional clauses in the foreach in
`DestroyRegionCommand.destroyRegion`: 1) a result contains a throwable 2) a
not-successfull result has a null message.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16246648#comment-16246648
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner commented on a change in pull request #1040: GEODE-3539: cleanup
GfshCommand and refactor tests
URL: https://github.com/apache/geode/pull/1040#discussion_r150099937
##
File path:
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandTest.java
##
@@ -144,4 +159,55 @@ private void addJvmOptionsForOutOfMemoryErrors(final
List commandLine) {
commandLine.add("-XXexitOnOutOfMemory");
}
}
+
+ private static final String FAKE_HOSTNAME = "someFakeHostname";
+
+ @Rule
+ public GfshParserRule commandRule = new GfshParserRule();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private StartLocatorCommand spy;
+
+ @Before
+ public void before() throws Exception {
+spy = Mockito.spy(StartLocatorCommand.class);
+doReturn(mock(Gfsh.class)).when(spy).getGfsh();
+ }
+
Review comment:
I'd prefer to see these new @Rule & @Before methods moved to the top of the
file with the original @Before method.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16246650#comment-16246650
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner commented on a change in pull request #1040: GEODE-3539: cleanup
GfshCommand and refactor tests
URL: https://github.com/apache/geode/pull/1040#discussion_r150098870
##
File path:
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandTest.java
##
@@ -144,4 +159,55 @@ private void addJvmOptionsForOutOfMemoryErrors(final
List commandLine) {
commandLine.add("-XXexitOnOutOfMemory");
}
}
+
+ private static final String FAKE_HOSTNAME = "someFakeHostname";
+
+ @Rule
+ public GfshParserRule commandRule = new GfshParserRule();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
Review comment:
Unused rule, should be deleted along with the import for `TemporaryFolder`
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16246649#comment-16246649
]
ASF GitHub Bot commented on GEODE-3539:
---
pdxrunner commented on a change in pull request #1040: GEODE-3539: cleanup
GfshCommand and refactor tests
URL: https://github.com/apache/geode/pull/1040#discussion_r150100257
##
File path:
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartServerCommandTest.java
##
@@ -141,4 +156,54 @@ public void testCreateServerCommandLineWithRestAPI()
throws Exception {
assertTrue(String.format("Expected ([]); but was (%1$s)",
expectedCommandLineElements),
expectedCommandLineElements.isEmpty());
}
+
+ private static final String FAKE_HOSTNAME = "someFakeHostname";
+
+ @Rule
+ public GfshParserRule commandRule = new GfshParserRule();
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ private StartServerCommand spy;
+
+ @Before
+ public void before() throws Exception {
+spy = Mockito.spy(StartServerCommand.class);
+doReturn(mock(Gfsh.class)).when(spy).getGfsh();
+ }
Review comment:
See above comments on StartLocatorCommandTest
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16243193#comment-16243193
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1024: GEODE-3539: Restore
test coverage for 'describe connection' command.
URL: https://github.com/apache/geode/pull/1024#discussion_r149549846
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
##
@@ -49,13 +49,17 @@
public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
@Test
- public void executeWhileConnected() throws Exception {
-// Depending on configuration, the host may resolve its local IP
-// to the alias present in /etc/hosts or use its internal host name.
-// Check for these first.
-gfsh.connectAndVerify(locator);
-gfsh.executeAndAssertThat("describe connection")
-.tableHasColumnWithValuesContaining("Connection Endpoints",
gfsh.getGfsh().getOperationInvoker().toString());
+ public void describeJmxConnection() throws Exception {
+gfsh.connectAndVerify(locator.getJmxPort(),
GfshShellConnectionRule.PortType.jmxManager);
+gfsh.executeAndAssertThat("describe
connection").tableHasColumnWithValuesContaining(
+"Connection Endpoints",
gfsh.getGfsh().getOperationInvoker().toString());
+ }
+
+ @Test
+ public void describeHttpConnection() throws Exception {
Review comment:
if this test is not in geode-web or geode-assembly, I think this would fail.
I don't think it would necessary to test http. the assertions are all the same.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16242951#comment-16242951
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #1024: GEODE-3539: Restore
test coverage for 'describe connection' command.
URL: https://github.com/apache/geode/pull/1024#discussion_r149512169
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
##
@@ -0,0 +1,103 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.Collections;
+import java.util.Enumeration;
+
+import org.apache.logging.log4j.Logger;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+/**
+ * The GfshCommandJUnitTest class is a test suite of test cases testing the
contract and
+ * functionality of the GfshCommand class for implementing GemFire shell
(Gfsh) commands.
+ *
+ * @see org.apache.geode.management.internal.cli.commands.GfshCommand
+ * @see org.jmock.Expectations
+ * @see org.jmock.Mockery
+ * @see org.jmock.lib.legacy.ClassImposteriser
+ * @see org.junit.Assert
+ * @see org.junit.Test
+ * @since GemFire 7.0
+ */
+
+@Category(IntegrationTest.class)
+public class DescribeConnectionCommandJUnitTest {
+ public static Logger logger = LogService.getLogger();
+
+ @ClassRule
+ public static LocatorStarterRule locator = new
LocatorStarterRule().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void executeWhileConnected() throws Exception {
+// Depending on configuration, the host may resolve its local IP
+// to the alias present in /etc/hosts or use its internal host name.
+// Check for these first.
+gfsh.connectAndVerify(locator);
+try {
Review comment:
That's much simpler! Good call.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16243012#comment-16243012 ] ASF subversion and git services commented on GEODE-3539: Commit d1fc1347f1411429594a9fde308d4b55a5afa966 in geode's branch refs/heads/feature/GEODE-3940 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=d1fc134 ] GEODE-3539: LocatorServerStartupRule enhancement (#1021) * get rid of the redundant variable that refers to either a LocatorStarter or a ServerStarter * add methods in MemberVM to wait till the region beans are ready * add convenient methods to get cache and servers and locators > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16242815#comment-16242815
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao commented on a change in pull request #1024: GEODE-3539: Restore
test coverage for 'describe connection' command.
URL: https://github.com/apache/geode/pull/1024#discussion_r149493281
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
##
@@ -0,0 +1,103 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.Collections;
+import java.util.Enumeration;
+
+import org.apache.logging.log4j.Logger;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+/**
+ * The GfshCommandJUnitTest class is a test suite of test cases testing the
contract and
+ * functionality of the GfshCommand class for implementing GemFire shell
(Gfsh) commands.
+ *
+ * @see org.apache.geode.management.internal.cli.commands.GfshCommand
+ * @see org.jmock.Expectations
+ * @see org.jmock.Mockery
+ * @see org.jmock.lib.legacy.ClassImposteriser
+ * @see org.junit.Assert
+ * @see org.junit.Test
+ * @since GemFire 7.0
+ */
+
+@Category(IntegrationTest.class)
+public class DescribeConnectionCommandJUnitTest {
+ public static Logger logger = LogService.getLogger();
+
+ @ClassRule
+ public static LocatorStarterRule locator = new
LocatorStarterRule().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void executeWhileConnected() throws Exception {
+// Depending on configuration, the host may resolve its local IP
+// to the alias present in /etc/hosts or use its internal host name.
+// Check for these first.
+gfsh.connectAndVerify(locator);
+try {
Review comment:
I ran the command with different connection type, and the results shows up
differently:
> connect --locator=localhost[60303]
Connecting to Locator at [host=localhost, port=60303] ..
Connecting to Manager at [host=10.118.33.214, port=21535] ..
Successfully connected to: [host=10.118.33.214, port=21535]
> describe connection
Connection Endpoints
10.118.33.214[21535]
> disconnect
Disconnecting from: 10.118.33.214[21535]
Disconnected from : 10.118.33.214[21535]
> connect --jmx-manager=localhost[21535]
Connecting to Manager at [host=localhost, port=21535] ..
Successfully connected to: [host=localhost, port=21535]
> describe connection
Connection Endpoints
localhost[21535]
> disconnect
Disconnecting from: localhost[21535]
Disconnected from : localhost[21535]
> connect --use-http
Successfully connected to: GemFire Manager HTTP service @
http://localhost:28478/geode-mgmt/v1
> describe connection
Connection Endpoints
---
GemFire Manager HTTP service @ http://localhost:28478/geode-mgmt/v1
we will need to have tests for each of these case.
It probably would be better if we would just assert with
gfsh.getInvoker().toString (which gives us the connection string).
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:
us...@infra.apache.org
> Add more test coverage for
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16242756#comment-16242756 ] ASF GitHub Bot commented on GEODE-3539: --- jinmeiliao opened a new pull request #1025: GEODE-3539: add more tests for get, put, locate entry commands URL: https://github.com/apache/geode/pull/1025 * clean up more methods in CliUtil * refactor tests out of GemfireDataCommandsDUnitTest * get rid of unnecessary try/catch block in commands Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16242691#comment-16242691 ] ASF GitHub Bot commented on GEODE-3539: --- PurelyApplied opened a new pull request #1024: GEODE-3539: Restore test coverage for 'describe connection' command. URL: https://github.com/apache/geode/pull/1024 Due to a critical oversight on my part, the test committed in ``a1b9725a7cb351299a5239a79f96daed9d88865b` and reverted in `70b4327b196630c638a6e7aef608405d2d73db98` did not account for testing environments that resolve the host address to an alias given in `/etc/hosts`. This commit restores the test, accounting for this possible output. Precheckin running. -- Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16242368#comment-16242368 ] ASF subversion and git services commented on GEODE-3539: Commit d1fc1347f1411429594a9fde308d4b55a5afa966 in geode's branch refs/heads/develop from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=d1fc134 ] GEODE-3539: LocatorServerStartupRule enhancement (#1021) * get rid of the redundant variable that refers to either a LocatorStarter or a ServerStarter * add methods in MemberVM to wait till the region beans are ready * add convenient methods to get cache and servers and locators > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16242367#comment-16242367
]
ASF GitHub Bot commented on GEODE-3539:
---
jinmeiliao closed pull request #1021: GEODE-3539: LocatorServerStartupRule
enhancement
URL: https://github.com/apache/geode/pull/1021
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-core/src/test/java/org/apache/geode/internal/cache/extension/ExtensionClusterConfigurationDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/extension/ExtensionClusterConfigurationDUnitTest.java
index c8eb03f1ec..8fb5feb420 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/extension/ExtensionClusterConfigurationDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/extension/ExtensionClusterConfigurationDUnitTest.java
@@ -18,8 +18,6 @@
import static org.apache.geode.test.dunit.Assert.assertTrue;
import static org.junit.Assert.assertEquals;
-import java.io.Serializable;
-
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -88,7 +86,7 @@ public void testCreateExtensions() throws Exception {
// Verify the config creation on this member
MemberVM newMember = locatorServerStartupRule.startServerVM(2,
locator.getPort());
newMember.invoke(() -> {
- InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+ InternalCache cache = LocatorServerStartupRule.getCache();
assertNotNull(cache);
Region region1 = cache.getRegion(REPLICATE_REGION);
@@ -143,7 +141,7 @@ public void testDestroyExtensions() throws Exception {
// Verify the config creation on this member
MemberVM newMember = locatorServerStartupRule.startServerVM(2,
locator.getPort());
newMember.invoke(() -> {
- InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+ InternalCache cache = LocatorServerStartupRule.getCache();
assertNotNull(cache);
Region region1 = cache.getRegion(REPLICATE_REGION);
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
index 5afd11298d..7b8a5e8286 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
@@ -18,11 +18,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
-import org.json.JSONArray;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -36,7 +32,6 @@
import org.apache.geode.compression.SnappyCompressor;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.cache.RegionEntryContext;
-import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.test.compiler.JarBuilder;
import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
@@ -78,7 +73,7 @@ public void testCreateRegionWithGoodCompressor() throws
Exception {
.statusIsSuccess();
server.invoke(() -> {
- Cache cache = LocatorServerStartupRule.serverStarter.getCache();
+ Cache cache = LocatorServerStartupRule.getCache();
Region region = cache.getRegion(regionName);
assertThat(region).isNotNull();
assertThat(region.getAttributes().getCompressor())
@@ -94,7 +89,7 @@ public void testCreateRegionWithBadCompressor() throws
Exception {
.statusIsError();
server.invoke(() -> {
- Cache cache = LocatorServerStartupRule.serverStarter.getCache();
+ Cache cache = LocatorServerStartupRule.getCache();
Region region = cache.getRegion(regionName);
assertThat(region).isNull();
});
@@ -107,7 +102,7 @@ public void testCreateRegionWithNoCompressor() throws
Exception {
.statusIsSuccess();
server.invoke(() -> {
- Cache cache = LocatorServerStartupRule.serverStarter.getCache();
+ Cache cache = LocatorServerStartupRule.getCache();
Region region = cache.getRegion(regionName);
assertThat(region).isNotNull();
assertThat(region.getAttributes().getCompressor()).isNull();
@@ -133,7 +128,7 @@ public void testCreateRegionWithPartitionResolver() throws
Exception {
.statusIsSuccess();
server.invoke(() -> {
- Cache cache =
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16241290#comment-16241290 ] ASF subversion and git services commented on GEODE-3539: Commit 50eb158cd863849e6546081942bff8876dda29c6 in geode's branch refs/heads/feature/GEODE-3930 from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=50eb158 ] Geode 3539: Add missing test coverage for 'version' command. (#1009) * GEODE-3539: Add missing test coverage for 'version' command. * Extract inner class GemFireVersion.VersionDescription to its own class * Made static strings in this this class visible for internal use. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16240868#comment-16240868 ] ASF subversion and git services commented on GEODE-3539: Commit 70b4327b196630c638a6e7aef608405d2d73db98 in geode's branch refs/heads/develop from [~prhomberg] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=70b4327 ] Revert "GEODE-3539: Add missing test coverage for 'describe connection' command." * Some test environments will expect the output to contain the alias given in /etc/hosts * This reverts commit a1b9725a7cb351299a5239a79f96daed9d88865b. > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16240678#comment-16240678
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied closed pull request #797: GEODE-3539: Add missing test coverage
for 'describe connection' command.
URL: https://github.com/apache/geode/pull/797
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-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
new file mode 100644
index 00..ca93a02332
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
@@ -0,0 +1,90 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.SocketException;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.StreamSupport;
+
+import com.google.common.collect.Iterators;
+import org.apache.logging.log4j.Logger;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+/**
+ * The GfshCommandJUnitTest class is a test suite of test cases testing the
contract and
+ * functionality of the GfshCommand class for implementing GemFire shell
(Gfsh) commands.
+ *
+ * @see org.apache.geode.management.internal.cli.commands.GfshCommand
+ * @see org.jmock.Expectations
+ * @see org.jmock.Mockery
+ * @see org.jmock.lib.legacy.ClassImposteriser
+ * @see org.junit.Assert
+ * @see org.junit.Test
+ * @since GemFire 7.0
+ */
+
+@Category(IntegrationTest.class)
+public class DescribeConnectionCommandJUnitTest {
+ public static Logger logger = LogService.getLogger();
+
+ @ClassRule
+ public static LocatorStarterRule locator = new
LocatorStarterRule().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void executeWhileConnected() throws Exception {
+gfsh.connectAndVerify(locator);
+// We must be sure to catch either IPv4 or IPv6 descriptions.
+String[] acceptableAddresses = getNetworkAddressArray();
+logger.info(
+"Expecting one of the following addresses: " + String.join(", ",
acceptableAddresses));
+gfsh.executeAndAssertThat("describe connection")
+.tableHasColumnWithValueMatchingOneOf("Connection Endpoints",
acceptableAddresses);
+ }
+
+ private String[] getNetworkAddressArray() throws SocketException {
+Enumeration networkInterfaces =
NetworkInterface.getNetworkInterfaces();
+NetworkInterface myInterface = networkInterfaces.nextElement();
+Enumeration myAddresses = myInterface.getInetAddresses();
+return
Collections.list(myAddresses).stream().map(InetAddress::getHostAddress)
+.map(address -> formatAddressAndPort(address,
locator.getJmxPort())).toArray(String[]::new);
+ }
+
+ @Test
+ public void executeWhileNotConnected() throws Exception {
+gfsh.executeAndAssertThat("describe connection")
+.tableHasColumnWithValuesContaining("Connection Endpoints", "Not
connected");
+ }
+
+ private String formatAddressAndPort(String address, int port) {
+address = address.startsWith("/") ? address.substring(1) : address;
+return address + "[" + port + "]";
+ }
+}
diff --git
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237842#comment-16237842
]
ASF GitHub Bot commented on GEODE-3539:
---
PurelyApplied commented on a change in pull request #797: GEODE-3539: Add
missing test coverage for 'describe connection' command.
URL: https://github.com/apache/geode/pull/797#discussion_r148830916
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.SocketException;
+import java.util.Enumeration;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.StreamSupport;
+
+import com.google.common.collect.Iterators;
+import org.apache.logging.log4j.Logger;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+/**
+ * The GfshCommandJUnitTest class is a test suite of test cases testing the
contract and
+ * functionality of the GfshCommand class for implementing GemFire shell
(Gfsh) commands.
+ *
+ * @see org.apache.geode.management.internal.cli.commands.GfshCommand
+ * @see org.jmock.Expectations
+ * @see org.jmock.Mockery
+ * @see org.jmock.lib.legacy.ClassImposteriser
+ * @see org.junit.Assert
+ * @see org.junit.Test
+ * @since GemFire 7.0
+ */
+
+@Category(IntegrationTest.class)
+public class DescribeConnectionCommandJUnitTest {
+ public static Logger logger = LogService.getLogger();
+
+ @ClassRule
+ public static LocatorStarterRule locator = new
LocatorStarterRule().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void executeWhileConnected() throws Exception {
+gfsh.connectAndVerify(locator);
+// We must be sure to catch either IPv4 or IPv6 descriptions.
+String[] acceptableAddresses = getNetworkAddressArray();
+logger.info(
+"Expecting one of the following addresses: " + String.join(", ",
acceptableAddresses));
+gfsh.executeAndAssertThat("describe connection")
+.tableHasColumnWithValueMatchingOneOf("Connection Endpoints",
acceptableAddresses);
+ }
+
+ private String[] getNetworkAddressArray() throws SocketException {
+Enumeration networkInterfaces =
NetworkInterface.getNetworkInterfaces();
+NetworkInterface myInterface = networkInterfaces.nextElement();
+Enumeration myAddresses = myInterface.getInetAddresses();
+return StreamSupport
Review comment:
That's much less cudgel-y! Thanks.
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>Priority: Major
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16236627#comment-16236627
]
ASF GitHub Bot commented on GEODE-3539:
---
jaredjstewart commented on a change in pull request #797: GEODE-3539: Add
missing test coverage for 'describe connection' command.
URL: https://github.com/apache/geode/pull/797#discussion_r148664747
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConnectionCommandJUnitTest.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.SocketException;
+import java.util.Enumeration;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.StreamSupport;
+
+import com.google.common.collect.Iterators;
+import org.apache.logging.log4j.Logger;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+/**
+ * The GfshCommandJUnitTest class is a test suite of test cases testing the
contract and
+ * functionality of the GfshCommand class for implementing GemFire shell
(Gfsh) commands.
+ *
+ * @see org.apache.geode.management.internal.cli.commands.GfshCommand
+ * @see org.jmock.Expectations
+ * @see org.jmock.Mockery
+ * @see org.jmock.lib.legacy.ClassImposteriser
+ * @see org.junit.Assert
+ * @see org.junit.Test
+ * @since GemFire 7.0
+ */
+
+@Category(IntegrationTest.class)
+public class DescribeConnectionCommandJUnitTest {
+ public static Logger logger = LogService.getLogger();
+
+ @ClassRule
+ public static LocatorStarterRule locator = new
LocatorStarterRule().withAutoStart();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Test
+ public void executeWhileConnected() throws Exception {
+gfsh.connectAndVerify(locator);
+// We must be sure to catch either IPv4 or IPv6 descriptions.
+String[] acceptableAddresses = getNetworkAddressArray();
+logger.info(
+"Expecting one of the following addresses: " + String.join(", ",
acceptableAddresses));
+gfsh.executeAndAssertThat("describe connection")
+.tableHasColumnWithValueMatchingOneOf("Connection Endpoints",
acceptableAddresses);
+ }
+
+ private String[] getNetworkAddressArray() throws SocketException {
+Enumeration networkInterfaces =
NetworkInterface.getNetworkInterfaces();
+NetworkInterface myInterface = networkInterfaces.nextElement();
+Enumeration myAddresses = myInterface.getInetAddresses();
+return StreamSupport
Review comment:
Since this Enumeration will be reliably bounded (finite), I think we can
turn it into a stream more concisely:
```
Collections.list(myAddresses).stream()
.map(InetAddress::getHostAddress)
.map(address -> formatAddressAndPort(address,
locator.getJmxPort())).toArray(String[]::new);
```
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>Priority: Major
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16236629#comment-16236629 ] ASF GitHub Bot commented on GEODE-3539: --- jaredjstewart commented on issue #797: GEODE-3539: Add missing test coverage for 'describe connection' command. URL: https://github.com/apache/geode/pull/797#issuecomment-341563669 Looks good overall! 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: us...@infra.apache.org > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao >Priority: Major > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16236626#comment-16236626
]
ASF GitHub Bot commented on GEODE-3539:
---
jaredjstewart commented on a change in pull request #797: GEODE-3539: Add
missing test coverage for 'describe connection' command.
URL: https://github.com/apache/geode/pull/797#discussion_r148665144
##
File path:
geode-core/src/test/java/org/apache/geode/test/junit/assertions/GfshShellConnectionRuleAssert.java
##
@@ -208,6 +208,31 @@ public GfshShellConnectionRuleAssert
tableHasColumnWithValuesContaining(String h
return this;
}
+ public GfshShellConnectionRuleAssert
tableHasColumnWithValueMatchingOneOf(String header,
+ String... acceptedValues) {
+GfJsonObject resultContentJSON = actual.getCommandResult().getContent();
+Object content = resultContentJSON.get(header);
+
+if (content == null) {
+ failWithMessage("Command result did not contain a table with column
header <" + header + ">: "
+ + resultContentJSON.toString());
+}
+
+Object[] actualValues = toArray((JSONArray) content);
+
+for (Object actualValue : actualValues) {
+ String actualValueString = (String) actualValue;
+ boolean actualValueContainsAnAcceptedValue =
+ Arrays.stream(acceptedValues).anyMatch(actualValueString::contains);
+
+ if (actualValueContainsAnAcceptedValue) {
+return this;
+ }
+}
+failWithMessage("No accepted value found.");
+return null;
Review comment:
I think you can still return `this` rather than returning `null`. (For
instance, someone might be using a JUnit rule that gathers up exceptions and
re-throws them at the end of a sequence of operations. We wouldn't want to
give them a NPE here in that case.)
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:
us...@infra.apache.org
> Add more test coverage for p2p commands
> ---
>
> Key: GEODE-3539
> URL: https://issues.apache.org/jira/browse/GEODE-3539
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
>Reporter: Jinmei Liao
>Priority: Major
>
> Add more command tests that would eventually get rid of the legacy tests.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (GEODE-3539) Add more test coverage for p2p commands
[ https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16227611#comment-16227611 ] ASF subversion and git services commented on GEODE-3539: Commit 19e5f8ce4995257fded70817436c3e8740115dd0 in geode's branch refs/heads/feature/GEODE-3637 from [~jinmeiliao] [ https://gitbox.apache.org/repos/asf?p=geode.git;h=19e5f8c ] GEODE-3539: Consolidate CliUtil and DataCommandUtils, DataCommandsUtil (#992) * GEODE-3539: Consolidate CliUtil and DataCommandUtils, DataCommandsUtil * remove DataCommandsUtil * move method from DataCommandUtils to CliUtil if applicable. * move moethds from DataCommandUtils to individual command if only used by one command > Add more test coverage for p2p commands > --- > > Key: GEODE-3539 > URL: https://issues.apache.org/jira/browse/GEODE-3539 > Project: Geode > Issue Type: Improvement > Components: gfsh >Reporter: Jinmei Liao > > Add more command tests that would eventually get rid of the legacy tests. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
