> On March 30, 2017, 10:44 p.m., Jared Stewart wrote: > > 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/diff/3/?file=1681140#file1681140line35> > > > > 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.
Good suggestion. Also, for the absolute path test we should be able to resolve the TemporaryFolder to an absolute path and use that as the root of the subdirectory instead of arbitarily using /tmp. - Ken ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58050/#review170648 ----------------------------------------------------------- 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 > >
