----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review172124 -----------------------------------------------------------
Ship it! Ship It! - Kirk Lund On April 13, 2017, 10:13 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58388/ > ----------------------------------------------------------- > > (Updated April 13, 2017, 10:13 p.m.) > > > Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick > Rhomberg. > > > Repository: geode > > > Description > ------- > > Finally the reviewboard is behaving now. This is good for review now. I will > close the PR. > > * consolidated the two sets of rules. > * do not allow member start up at test initialization time. > * validate properties in @Before > * use provider in the chained rules to get the appropriate ports in @Before > > Jared and I tried to use ServerLauncher/LocatorLauncher to start the > respective member in the rules, but precheckin yields many failures. Looks > like it has two problems: > 1) by passing in the workingDir in the launcher alone does not make all the > logs go there. I still have to set the user.dir env variable to make some > tests pass. > 2) launcher.stop does not stop the cache/locator/server cleanly. Subsequent > tests fail due to insufficient cleanup. > Due to the above two reasons, I reverted back to use the old way to start > server/locator. This looks like a lean and mean way to get what we needed. > > We also investigated the use of Builder in the rules. At least for now, it > doesn't buy us much since we need to do validation/startup servers in @Before > of the rules. And it yeilds some duplication code and make the usage not that > intuitive. > > As for using AvailablePort.Keeper, Jared and I found out it doesn't really > keep the ports, so revert that back as well. > > > Diffs > ----- > > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java > 4d142bd6b7aa91b162a4fdf4e546df2d3285290e > > geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java > e41d0fea9a1a89ec247f3f2750cc944083056a87 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java > dc24a57dcf3243962c27d349da136f09d49b1250 > > > Diff: https://reviews.apache.org/r/58388/diff/4/ > > > Testing > ------- > > precheckin successful. > > > Thanks, > > Jinmei Liao > >
