[GitHub] incubator-geode pull request #297: GEODE-734: gfsh export stack-traces shoul...

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

Github user kirklund commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/297#discussion_r90507113
  
    --- Diff: geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowStackTraceDUnitTest.java ---
    @@ -149,4 +163,92 @@ 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 when file is already present
    +   *
    +   * @throws ClassNotFoundException
    +   * @throws IOException
    +   */
    +  @Test
    --- End diff --
    
    This looks like three different tests all inside one test. It's better to separate these so they're shorter, more readable and pass or fail independently of one another.
    
    If you're worried about how long it takes to setup/teardown or run tests, then you really should consider testing this feature in a UnitTest or IntegrationTest.
    
    UnitTest would use Mockito to help you isolate the one class for test (UnitTest shouldn't touch file system or network). IntegrationTest would allow you to touch file system or network, test group of classes, or create one Geode member (in one JVM). DistributedTest should only be used when you really need more than one member (multiple JVMs).


---
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.
---