geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jinmei Liao <jil...@pivotal.io>
Subject Re: Review Request 58388: GEODE-2730: refactor rules
Date Thu, 13 Apr 2017 22:13:51 GMT

-----------------------------------------------------------
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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/58388/diff/3-4/


Testing
-------

precheckin successful.


Thanks,

Jinmei Liao


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message