----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58050/#review170648 -----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java Line 32 (original), 35 (patched) <https://reviews.apache.org/r/58050/#comment243539> Let's change this to a `@Rule` instead of a `@ClassRule`. This will cause the locator to be created and destroyed for every `@Test`, rather than once for the whole class. The all of the `try{} finally{}` blocks below will be unnecessary. The `LocatorStarterRule` uses a `TemporaryFolder` to back the locator's working dir that will automatically be deleted by the `@Rule`'s lifecycle. geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java Line 38 (original), 41 (patched) <https://reviews.apache.org/r/58050/#comment243540> If we add the `@Before` annotation to this method, JUnit will automatically invoke it before every `@Test` method. Then we can delete the repetitive calls to `connect()` at the beginning of each `@Test` method below. geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java Lines 81 (patched) <https://reviews.apache.org/r/58050/#comment243538> This way of resolving the subdirectory will not work on any OS that does not use "/" as a file separator (i.e. Windows). Try this instead: ``` Path workingDirPath = locator.getWorkingDir().toPath(); Path subdirPath = workingDirPath.resolve("some").resolve("test").resolve("directory"); String relativeDir = workingDirPath.relativize(subdirPath).toString(); gfsh.executeCommand("export logs --dir=some/test/directory"); assertThat(FileUtils.listFiles(subdirPath.toFile(), extensions, false)).isNotEmpty(); assertThat(FileUtils.listFiles(locator.getWorkingDir(), extensions, false)).isEmpty(); ``` - Jared Stewart On March 30, 2017, 6:17 p.m., Patrick Rhomberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58050/ > ----------------------------------------------------------- > > (Updated March 30, 2017, 6:17 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Kevin Duling, Ken Howe, > Kirk Lund, and Swapnil Bawaskar. > > > Repository: geode > > > Description > ------- > > export logs --dir refers to local filesystem when connected via HTTP and > refers to the managers filesystem when connected via JMX. This behavior will > be changed in GEODE-2663. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java > 3f147c19a128dce78c51c31e6758e517cd2ab496 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java > 5b1f089c18c404f64929398f6015839eb783ccb4 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java > 268fa397db253f12c0effdbf6faa5e822730144c > > geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties > 3c56def01ad58f98ea1707f4dd6b57e643e9eab1 > > > Diff: https://reviews.apache.org/r/58050/diff/3/ > > > Testing > ------- > > precheckin running. > > Manual verification of behavior both with and without --dir option, both > connected via HTTP and not. > > > Thanks, > > Patrick Rhomberg > >
