[GitHub] geode pull request #297: GEODE-734: gfsh export stack-traces should not requ...

[kirklund <gi...@git.apache.org>]

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---