Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-16 Thread Galen O'Sullivan
I have a PR up to deprecate these classes and several other distributed test classes that use inheritance (I actually forgot to mention it here when I first put it up, whoops): https://github.com/apache/geode/pull/2841 I intend to merge it Monday evening if there are no objections. I think I'll

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-12 Thread Jinmei Liao
I feel like we are mixing two topic in this discussion. One is to improve our user API, and one is to write clean tests. Doing one doesn't have to sacrifice the other. If our rules are using internal APIs, then it's a chance to check if we can improve our APIs like Kirk has done. But stop using

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-12 Thread Juan José Ramos
Hello all, What about mixing both approaches?. We can use the *Rules* to avoid duplication of code *but re-write them* to directly use the Geode User APIs instead of the Geode Internal API. Just for the record, https://issues.apache.org/jira/browse/GEODE-5739 was created for something similar

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Jinmei Liao
Yeah, I guess. But why going out of this way to avoid using rules? Why rules are bad? Rules just encapsulate common befores and afters to avoid duplicate code in every tests. I don't see why we should avoid using it. On Fri, Nov 9, 2018, 3:44 PM Galen O'Sullivan I wonder if we could use some

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Galen O'Sullivan
I wonder if we could use some sort of assertions to validate that that tests have cleaned up, at least in a few ways? For example, if a Cache/Locator/DistributedSystem is still running after a test has finished, that's a good sign of a dirty environment. If system properties are still set, that's

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Kirk Lund
I would love to see you apply some of your passion for that to improving the User APIs so there's less boiler plate code for the Users as well. Please don't forget that to Developers who are not familiar with our Rules such as ClusterStarterRule, that it can be very difficult to understand what

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Jinmei Liao
I like using the rules because it reduces boiler plate code and the chance of not cleaning up properly after your tests. It also make what you are really trying to do in the test stand out more in the test code. On Fri, Nov 9, 2018 at 1:37 PM Kirk Lund wrote: > We need to pull out the DUnit

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Kirk Lund
We need to pull out the DUnit Blackboard from DistributedTestCase and repackage it as a BlackboardRule. It makes sense to make that a JUnit Rule because it's not part of Geode or its User APIs but it's really useful for distributed testing in general. It's also probably the last useful thing

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Bruce Schuchardt
I agree with Kirk about the Rules and I agree with Galen about moving away from the old abstract test classes.  I think that is also in the spirit of what Kirk is saying. There are also tests that have complicated methods for creating caches and regions.  These methods have many parameters

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Kirk Lund
*I would like to encourage all Geode developers to start writing tests directly against the Geode User APIs* even in DistributedTests. I'm no longer using *CacheRule, ClientCacheRule, ServerStarterRule, LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging their use any longer.

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Jinmei Liao
+1 on this. We should break away from extending base test class now. It makes it quite hard to understand how the test cluster is set up and what the test is doing. On Fri, Nov 9, 2018 at 10:50 AM Galen O'Sullivan wrote: > I was looking at a test recently that extended JUnit4CacheTestCase and

[DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

2018-11-09 Thread Galen O'Sullivan
I was looking at a test recently that extended JUnit4CacheTestCase and only used a single server, and changed it to use ClusterStartupRule. JUnit4CacheTestCase adds additional complexity to JUnit4DistributedTestCase and with the move to ClusterStartupRule for distributed tests, rather than class