Github user kirklund commented on a diff in the pull request:
https://github.com/apache/geode/pull/297#discussion_r91152153
--- Diff:
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowStackTraceDUnitTest.java
---
@@ -149,4 +143,126 @@ public void testExportStacktrace() throws
ClassNotFoundException, IOException {
getLogWriter().info("Output : \n" +
commandResultToString(commandResult));
assertFalse(commandResult.getStatus().equals(Status.OK));
}
+
+ /***
+ * Tests the behavior of the show stack-trace command to verify that
files with any extension are
+ * allowed Refer: GEODE-734
+ *
+ * @throws ClassNotFoundException
+ * @throws IOException
+ */
+ @Test
+ public void testExportStacktraceWithNonTXTFile() throws
ClassNotFoundException, IOException {
+ setupSystem();
+
+ // Test non txt extension file is allowed
+ File stacktracesFile = workDirectory.newFile("allStackTraces.log");
+ CommandStringBuilder commandStringBuilder =
+ new CommandStringBuilder(CliStrings.EXPORT_STACKTRACE);
+ commandStringBuilder.addOption(CliStrings.EXPORT_STACKTRACE__FILE,
+ stacktracesFile.getCanonicalPath());
+ String exportCommandString = commandStringBuilder.toString();
+ getLogWriter().info("CommandString : " + exportCommandString);
+ CommandResult exportCommandResult =
executeCommand(exportCommandString);
+ getLogWriter().info("Output : \n" +
commandResultToString(exportCommandResult));
+ assertTrue(exportCommandResult.getStatus().equals(Status.OK));
+
+ // test file with-out any extension
+ File allStacktracesFile = workDirectory.newFile("allStackTraces");
+ commandStringBuilder = new
CommandStringBuilder(CliStrings.EXPORT_STACKTRACE);
+ commandStringBuilder.addOption(CliStrings.EXPORT_STACKTRACE__FILE,
+ allStacktracesFile.getCanonicalPath());
+ exportCommandString = commandStringBuilder.toString();
+ getLogWriter().info("CommandString : " + exportCommandString);
+ exportCommandResult = executeCommand(exportCommandString);
+ getLogWriter().info("Output : \n" +
commandResultToString(exportCommandResult));
+ assertTrue(exportCommandResult.getStatus().equals(Status.OK));
+ }
+
+ /***
+ * Tests the behavior of the show stack-trace command when file is
already present and
+ * abort-if-file-exists option is set to false(which is default). As a
result it should overwrite
+ * the file and return OK status
+ *
+ * @throws ClassNotFoundException
+ * @throws IOException
+ */
+ @Test
+ public void testExportStacktraceWhenFilePresent() throws
ClassNotFoundException, IOException {
+ setupSystem();
+
+ // test pass although file present
+ File stacktracesFile = workDirectory.newFile("allStackTraces.log");
+ CommandStringBuilder commandStringBuilder =
+ new CommandStringBuilder(CliStrings.EXPORT_STACKTRACE);
+ commandStringBuilder.addOption(CliStrings.EXPORT_STACKTRACE__FILE,
+ stacktracesFile.getCanonicalPath());
+ String exportCommandString = commandStringBuilder.toString();
+ getLogWriter().info("CommandString : " + exportCommandString);
+ CommandResult exportCommandResult =
executeCommand(exportCommandString);
+ getLogWriter().info("Output : \n" +
commandResultToString(exportCommandResult));
+ assertTrue(exportCommandResult.getStatus().equals(Status.OK));
+
+ }
+
+ /***
+ * Tests the behavior of the show stack-trace command when file is
already present and when
+ * abort-if-file-exists option is set to true. As a result it should
fail with ERROR status
+ *
+ * @throws ClassNotFoundException
+ * @throws IOException
+ */
+ @Test
+ public void testExportStacktraceFilePresentWithAbort()
+ throws ClassNotFoundException, IOException {
+ setupSystem();
+
+ File stacktracesFile = workDirectory.newFile("allStackTraces.log");
+ CommandStringBuilder commandStringBuilder =
+ new CommandStringBuilder(CliStrings.EXPORT_STACKTRACE);
+ commandStringBuilder.addOption(CliStrings.EXPORT_STACKTRACE__FILE,
+ stacktracesFile.getCanonicalPath());
+
commandStringBuilder.addOption(CliStrings.EXPORT_STACKTRACE__FAIL__IF__FILE__PRESENT,
+ Boolean.TRUE.toString());
+ String exportCommandString = commandStringBuilder.toString();
+ getLogWriter().info("CommandString : " + exportCommandString);
+ CommandResult exportCommandResult =
executeCommand(exportCommandString);
+ getLogWriter().info("Output : \n" +
commandResultToString(exportCommandResult));
+ assertTrue(exportCommandResult.getStatus().equals(Status.ERROR));
+ try {
+ assertTrue(((String)
exportCommandResult.getResultData().getGfJsonObject()
+ .getJSONObject("content").getJSONArray("message").get(0))
+ .contains("file " + stacktracesFile.getCanonicalPath() + "
already present"));
+ } catch (GfJsonException e) {
--- End diff --
I have one more change to request to the two tests with try-catch blocks.
Please change the method signature to throws Exception and delete the try-catch
block. JUnit handles this better than using fail with e.getCause() or
e.getMessage(). With JUnit handling it, it's much easier to debug if these
tests ever fail. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
