Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 try to use the public facing APIs the next time I write a new test and see how it goes. Thanks, Galen On Mon, Nov 12, 2018 at 8:41 AM Jinmei Liao wrote: > 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 the rules or change the rules to use a more verbose API should not > the way to go. My 2 cents. > > On Mon, Nov 12, 2018 at 4:22 AM Juan José Ramos wrote: > > > 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 (use [Server|Locator]Launcher instead of > > internal API in [Server|Locator]StarterRule), but it didn't get enough > > consent to be merged (leaving aside the huge amount of unrelated changes > in > > the PR, the idea itself was rejected). > > Cheers. > > > > > > On Fri, Nov 9, 2018 at 11:55 PM Jinmei Liao wrote: > > > > > 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 > > wrote: > > > > > > > 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 a good sign of a dirty environment. We could use a custom > > > test > > > > runner, or even add a rule to all our tests en masse that checks that > > > > things are cleaned up. > > > > > > > > Jinmei, for single-JVM tests, you could write a method for your test > > (or > > > > test class) that sets whatever properties you need and returns a > Cache > > > > constructed with those properties. Then you can use > try-with-resources > > to > > > > make sure that the Cache is properly closed. Is that a good > alternative > > > to > > > > using a rule? > > > > > > > > On Fri, Nov 9, 2018 at 3:28 PM Helena Bales > wrote: > > > > > > > > > +1 for deprecating old test bases. Many of the tests that gave me > the > > > > most > > > > > trouble this summer were because of JUnit4CacheTestCase. > > > > > Also +1 for pulling out Blackboard into a rule. > > > > > > > > > > I will, however, argue for continuing to use ClusterStartupRule. > The > > > > > benefit of that is that it makes sure that all JVMs started for > > servers > > > > and > > > > > locators are cleaned up at the end of the tests, even if the tests > > > fail. > > > > We > > > > > could certainly spend time making that code easier to understand, > > but I > > > > > don't think that starting clusters is straightforward enough to > have > > > > > confidence that it will get done correctly every time. > Multi-threaded > > > > tests > > > > > should be a cautionary tale for this; some implementations were > fine, > > > but > > > > > many polluted the system with threads that never stopped and tests > > that > > > > > didn't actually test anything. > > > > > As I see it, we are paying in readability for tests that do things > > the > > > > > right way. > > > > > > > > > > On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund wrote: > > > > > > > > > > > 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 it has done and how it has configured Geode. The more the > Rule > > > > does, > > > > > > the greater this problem. The fact that most of these Rules use > > > > Internal > > > > > > APIs instead of User APIs is also a problem in my opinion because > > > we're > > > > > not > > > > > > testing exactly what a User would do or can do. > > > > > > > > > > > > To many of us Developers, figuring out what all the rules have > > > > configured > > > > > > and done is a much bigger problem than it is to deal with verbose > > > code > > > > > in a > > > > > >
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 the rules or change the rules to use a more verbose API should not the way to go. My 2 cents. On Mon, Nov 12, 2018 at 4:22 AM Juan José Ramos wrote: > 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 (use [Server|Locator]Launcher instead of > internal API in [Server|Locator]StarterRule), but it didn't get enough > consent to be merged (leaving aside the huge amount of unrelated changes in > the PR, the idea itself was rejected). > Cheers. > > > On Fri, Nov 9, 2018 at 11:55 PM Jinmei Liao wrote: > > > 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 > wrote: > > > > > 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 a good sign of a dirty environment. We could use a custom > > test > > > runner, or even add a rule to all our tests en masse that checks that > > > things are cleaned up. > > > > > > Jinmei, for single-JVM tests, you could write a method for your test > (or > > > test class) that sets whatever properties you need and returns a Cache > > > constructed with those properties. Then you can use try-with-resources > to > > > make sure that the Cache is properly closed. Is that a good alternative > > to > > > using a rule? > > > > > > On Fri, Nov 9, 2018 at 3:28 PM Helena Bales wrote: > > > > > > > +1 for deprecating old test bases. Many of the tests that gave me the > > > most > > > > trouble this summer were because of JUnit4CacheTestCase. > > > > Also +1 for pulling out Blackboard into a rule. > > > > > > > > I will, however, argue for continuing to use ClusterStartupRule. The > > > > benefit of that is that it makes sure that all JVMs started for > servers > > > and > > > > locators are cleaned up at the end of the tests, even if the tests > > fail. > > > We > > > > could certainly spend time making that code easier to understand, > but I > > > > don't think that starting clusters is straightforward enough to have > > > > confidence that it will get done correctly every time. Multi-threaded > > > tests > > > > should be a cautionary tale for this; some implementations were fine, > > but > > > > many polluted the system with threads that never stopped and tests > that > > > > didn't actually test anything. > > > > As I see it, we are paying in readability for tests that do things > the > > > > right way. > > > > > > > > On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund wrote: > > > > > > > > > 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 it has done and how it has configured Geode. The more the Rule > > > does, > > > > > the greater this problem. The fact that most of these Rules use > > > Internal > > > > > APIs instead of User APIs is also a problem in my opinion because > > we're > > > > not > > > > > testing exactly what a User would do or can do. > > > > > > > > > > To many of us Developers, figuring out what all the rules have > > > configured > > > > > and done is a much bigger problem than it is to deal with verbose > > code > > > > in a > > > > > setUp method that uses CacheFactory directly. On one hand I want to > > > say, > > > > do > > > > > as you prefer but we also have to consider that other Developers > need > > > to > > > > > maintain these tests that are using the Rules, so I will continue > to > > > > > advocate for the writing of tests using Geode User APIs as much as > > > > possible > > > > > for the reasons I already stated. > > > > > > > > > > On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao > > wrote: > > > > > > > > > > > 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
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 (use [Server|Locator]Launcher instead of internal API in [Server|Locator]StarterRule), but it didn't get enough consent to be merged (leaving aside the huge amount of unrelated changes in the PR, the idea itself was rejected). Cheers. On Fri, Nov 9, 2018 at 11:55 PM Jinmei Liao wrote: > 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 wrote: > > > 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 a good sign of a dirty environment. We could use a custom > test > > runner, or even add a rule to all our tests en masse that checks that > > things are cleaned up. > > > > Jinmei, for single-JVM tests, you could write a method for your test (or > > test class) that sets whatever properties you need and returns a Cache > > constructed with those properties. Then you can use try-with-resources to > > make sure that the Cache is properly closed. Is that a good alternative > to > > using a rule? > > > > On Fri, Nov 9, 2018 at 3:28 PM Helena Bales wrote: > > > > > +1 for deprecating old test bases. Many of the tests that gave me the > > most > > > trouble this summer were because of JUnit4CacheTestCase. > > > Also +1 for pulling out Blackboard into a rule. > > > > > > I will, however, argue for continuing to use ClusterStartupRule. The > > > benefit of that is that it makes sure that all JVMs started for servers > > and > > > locators are cleaned up at the end of the tests, even if the tests > fail. > > We > > > could certainly spend time making that code easier to understand, but I > > > don't think that starting clusters is straightforward enough to have > > > confidence that it will get done correctly every time. Multi-threaded > > tests > > > should be a cautionary tale for this; some implementations were fine, > but > > > many polluted the system with threads that never stopped and tests that > > > didn't actually test anything. > > > As I see it, we are paying in readability for tests that do things the > > > right way. > > > > > > On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund wrote: > > > > > > > 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 it has done and how it has configured Geode. The more the Rule > > does, > > > > the greater this problem. The fact that most of these Rules use > > Internal > > > > APIs instead of User APIs is also a problem in my opinion because > we're > > > not > > > > testing exactly what a User would do or can do. > > > > > > > > To many of us Developers, figuring out what all the rules have > > configured > > > > and done is a much bigger problem than it is to deal with verbose > code > > > in a > > > > setUp method that uses CacheFactory directly. On one hand I want to > > say, > > > do > > > > as you prefer but we also have to consider that other Developers need > > to > > > > maintain these tests that are using the Rules, so I will continue to > > > > advocate for the writing of tests using Geode User APIs as much as > > > possible > > > > for the reasons I already stated. > > > > > > > > On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao > wrote: > > > > > > > > > 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 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 > > > > > > that's still in DistributedTestCase and no where else. > > > > > > > > > > > > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt < > > > > >
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 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 a good sign of a dirty environment. We could use a custom test > runner, or even add a rule to all our tests en masse that checks that > things are cleaned up. > > Jinmei, for single-JVM tests, you could write a method for your test (or > test class) that sets whatever properties you need and returns a Cache > constructed with those properties. Then you can use try-with-resources to > make sure that the Cache is properly closed. Is that a good alternative to > using a rule? > > On Fri, Nov 9, 2018 at 3:28 PM Helena Bales wrote: > > > +1 for deprecating old test bases. Many of the tests that gave me the > most > > trouble this summer were because of JUnit4CacheTestCase. > > Also +1 for pulling out Blackboard into a rule. > > > > I will, however, argue for continuing to use ClusterStartupRule. The > > benefit of that is that it makes sure that all JVMs started for servers > and > > locators are cleaned up at the end of the tests, even if the tests fail. > We > > could certainly spend time making that code easier to understand, but I > > don't think that starting clusters is straightforward enough to have > > confidence that it will get done correctly every time. Multi-threaded > tests > > should be a cautionary tale for this; some implementations were fine, but > > many polluted the system with threads that never stopped and tests that > > didn't actually test anything. > > As I see it, we are paying in readability for tests that do things the > > right way. > > > > On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund wrote: > > > > > 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 it has done and how it has configured Geode. The more the Rule > does, > > > the greater this problem. The fact that most of these Rules use > Internal > > > APIs instead of User APIs is also a problem in my opinion because we're > > not > > > testing exactly what a User would do or can do. > > > > > > To many of us Developers, figuring out what all the rules have > configured > > > and done is a much bigger problem than it is to deal with verbose code > > in a > > > setUp method that uses CacheFactory directly. On one hand I want to > say, > > do > > > as you prefer but we also have to consider that other Developers need > to > > > maintain these tests that are using the Rules, so I will continue to > > > advocate for the writing of tests using Geode User APIs as much as > > possible > > > for the reasons I already stated. > > > > > > On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao wrote: > > > > > > > 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 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 > > > > > that's still in DistributedTestCase and no where else. > > > > > > > > > > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt < > > > > bschucha...@pivotal.io> > > > > > wrote: > > > > > > > > > > > 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 and are sometimes in > > > > Helper > > > > > > classes. I've found these especially difficult to deal with when > > > > fixing > > > > > > flaky tests because changing one of the Helper methods affects > many > > > > > tests. > > > > > > > > > > > > > > > > > > On 11/9/18 11:31 AM, Kirk Lund wrote: > > > > > > > > > > > >> *I would like to encourage
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 a good sign of a dirty environment. We could use a custom test runner, or even add a rule to all our tests en masse that checks that things are cleaned up. Jinmei, for single-JVM tests, you could write a method for your test (or test class) that sets whatever properties you need and returns a Cache constructed with those properties. Then you can use try-with-resources to make sure that the Cache is properly closed. Is that a good alternative to using a rule? On Fri, Nov 9, 2018 at 3:28 PM Helena Bales wrote: > +1 for deprecating old test bases. Many of the tests that gave me the most > trouble this summer were because of JUnit4CacheTestCase. > Also +1 for pulling out Blackboard into a rule. > > I will, however, argue for continuing to use ClusterStartupRule. The > benefit of that is that it makes sure that all JVMs started for servers and > locators are cleaned up at the end of the tests, even if the tests fail. We > could certainly spend time making that code easier to understand, but I > don't think that starting clusters is straightforward enough to have > confidence that it will get done correctly every time. Multi-threaded tests > should be a cautionary tale for this; some implementations were fine, but > many polluted the system with threads that never stopped and tests that > didn't actually test anything. > As I see it, we are paying in readability for tests that do things the > right way. > > On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund wrote: > > > 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 it has done and how it has configured Geode. The more the Rule does, > > the greater this problem. The fact that most of these Rules use Internal > > APIs instead of User APIs is also a problem in my opinion because we're > not > > testing exactly what a User would do or can do. > > > > To many of us Developers, figuring out what all the rules have configured > > and done is a much bigger problem than it is to deal with verbose code > in a > > setUp method that uses CacheFactory directly. On one hand I want to say, > do > > as you prefer but we also have to consider that other Developers need to > > maintain these tests that are using the Rules, so I will continue to > > advocate for the writing of tests using Geode User APIs as much as > possible > > for the reasons I already stated. > > > > On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao wrote: > > > > > 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 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 > > > > that's still in DistributedTestCase and no where else. > > > > > > > > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt < > > > bschucha...@pivotal.io> > > > > wrote: > > > > > > > > > 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 and are sometimes in > > > Helper > > > > > classes. I've found these especially difficult to deal with when > > > fixing > > > > > flaky tests because changing one of the Helper methods affects many > > > > tests. > > > > > > > > > > > > > > > On 11/9/18 11:31 AM, Kirk Lund wrote: > > > > > > > > > >> *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. I'll explain why below. > > > > >> > > > > >> Here's an example for an IntegrationTest that needs a Cache but > not > > > any > > > > >> CacheServers: > >
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 it has done and how it has configured Geode. The more the Rule does,
the greater this problem. The fact that most of these Rules use Internal
APIs instead of User APIs is also a problem in my opinion because we're not
testing exactly what a User would do or can do.
To many of us Developers, figuring out what all the rules have configured
and done is a much bigger problem than it is to deal with verbose code in a
setUp method that uses CacheFactory directly. On one hand I want to say, do
as you prefer but we also have to consider that other Developers need to
maintain these tests that are using the Rules, so I will continue to
advocate for the writing of tests using Geode User APIs as much as possible
for the reasons I already stated.
On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao wrote:
> 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 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
> > that's still in DistributedTestCase and no where else.
> >
> > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt <
> bschucha...@pivotal.io>
> > wrote:
> >
> > > 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 and are sometimes in
> Helper
> > > classes. I've found these especially difficult to deal with when
> fixing
> > > flaky tests because changing one of the Helper methods affects many
> > tests.
> > >
> > >
> > > On 11/9/18 11:31 AM, Kirk Lund wrote:
> > >
> > >> *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. I'll explain why below.
> > >>
> > >> Here's an example for an IntegrationTest that needs a Cache but not
> any
> > >> CacheServers:
> > >>
> > >> private Cache cache;
> > >>
> > >> @Before
> > >> public void setUp() {
> > >>Properties config = new Properties();
> > >>config.setProperty(LOCATORS, "");
> > >>cache = new CacheFactory(config).create();
> > >> }
> > >>
> > >> @After
> > >> public void tearDown() {
> > >>cache.close();
> > >> }
> > >>
> > >> That's some pretty simple code and as a Developer, I can tell exactly
> > what
> > >> it's doing and what the config is.
> > >>
> > >> Here's an example of the kind of Geode User API code that I use to
> > create
> > >> Servers in a DistributedTests now:
> > >>
> > >>private void createServer(String serverName, File serverDir, int
> > >> locatorPort) {
> > >> ServerLauncher.Builder builder = new ServerLauncher.Builder();
> > >> builder.setMemberName(serverName);
> > >> builder.setWorkingDirectory(serverDir.getAbsolutePath());
> > >> builder.setServerPort(0);
> > >> builder.set(LOCATORS, "localHost[" + locatorPort + "]");
> > >> builder.set(DISABLE_AUTO_RECONNECT, "false");
> > >> builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
> > >> builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
> > >> builder.set(MEMBER_TIMEOUT, "2000");
> > >>
> > >> serverLauncher = builder.build();
> > >> serverLauncher.start();
> > >> assertThat(serverLauncher.isRunning()).isTrue();
> > >>}
> > >>
> > >> In particular, I think we should be using ServerLauncher and
> > >> LocatorLauncher instead of Rules when we want a full-stack Locator or
> > >> full-stack Server that looks like what a User is going to startup.
> > >>
> > >> Here are my reasons:
> > >>
> > >> 1) I want to learn and use the Geode User APIs directly, not someone's
> > >> (even mine) Testing API that hides the Geode User APIs. If I see a
> test
> > >> fail, I want to see exactly what was configured and what User APIs
> were
> > >> used right there in the test without having to open other classes. I
> > don't
> > >> want to have to spend even 15 minutes digging through some JUnit Rule
> to
> > >> figure out how
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 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
> that's still in DistributedTestCase and no where else.
>
> On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt
> wrote:
>
> > 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 and are sometimes in Helper
> > classes. I've found these especially difficult to deal with when fixing
> > flaky tests because changing one of the Helper methods affects many
> tests.
> >
> >
> > On 11/9/18 11:31 AM, Kirk Lund wrote:
> >
> >> *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. I'll explain why below.
> >>
> >> Here's an example for an IntegrationTest that needs a Cache but not any
> >> CacheServers:
> >>
> >> private Cache cache;
> >>
> >> @Before
> >> public void setUp() {
> >>Properties config = new Properties();
> >>config.setProperty(LOCATORS, "");
> >>cache = new CacheFactory(config).create();
> >> }
> >>
> >> @After
> >> public void tearDown() {
> >>cache.close();
> >> }
> >>
> >> That's some pretty simple code and as a Developer, I can tell exactly
> what
> >> it's doing and what the config is.
> >>
> >> Here's an example of the kind of Geode User API code that I use to
> create
> >> Servers in a DistributedTests now:
> >>
> >>private void createServer(String serverName, File serverDir, int
> >> locatorPort) {
> >> ServerLauncher.Builder builder = new ServerLauncher.Builder();
> >> builder.setMemberName(serverName);
> >> builder.setWorkingDirectory(serverDir.getAbsolutePath());
> >> builder.setServerPort(0);
> >> builder.set(LOCATORS, "localHost[" + locatorPort + "]");
> >> builder.set(DISABLE_AUTO_RECONNECT, "false");
> >> builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
> >> builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
> >> builder.set(MEMBER_TIMEOUT, "2000");
> >>
> >> serverLauncher = builder.build();
> >> serverLauncher.start();
> >> assertThat(serverLauncher.isRunning()).isTrue();
> >>}
> >>
> >> In particular, I think we should be using ServerLauncher and
> >> LocatorLauncher instead of Rules when we want a full-stack Locator or
> >> full-stack Server that looks like what a User is going to startup.
> >>
> >> Here are my reasons:
> >>
> >> 1) I want to learn and use the Geode User APIs directly, not someone's
> >> (even mine) Testing API that hides the Geode User APIs. If I see a test
> >> fail, I want to see exactly what was configured and what User APIs were
> >> used right there in the test without having to open other classes. I
> don't
> >> want to have to spend even 15 minutes digging through some JUnit Rule to
> >> figure out how PDX was configured.
> >>
> >> 2) We need to make sure that the Geode User APIs are easy to use and are
> >> complete. If we're writing tests against Testing APIs instead then we
> >> don't
> >> feel the Users' pain if our API is painful. If the reason to use a Rule
> is
> >> because our User API is overly-verbose of difficult, then that's even
> more
> >> reason to use the Geode User API, so we recognize that it needs to
> change!
> >>
> >> GemFire had a long history of hiding its User APIs behind elaborate
> >> Testing
> >> APIs and we all used these fancy, easier to use, more compact Testing
> >> APIs.
> >> This promotes complicated, inconsistent and potentially incomplete User
> >> APIs for Users to actually use. The result: difficult to use product
> with
> >> difficult to use APIs and User APIs that are missing important things
> that
> >> then Users have to resort to internal APIs to use. I'm strongly
> convinced
> >> that using elaborate Testing APIs is at least largely responsible for
> >> making GemFire and now Geode difficult to use and that's why I'm pushing
> >> so
> >> hard to write tests with Geode User APIs instead of convenient custom
> >> Rules
> >>
> >> Since I started using ServerLauncher and LocatorLauncher APIs directly
> in
> >> my DistributedTests I made a very important
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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
that's still in DistributedTestCase and no where else.
On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt
wrote:
> 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 and are sometimes in Helper
> classes. I've found these especially difficult to deal with when fixing
> flaky tests because changing one of the Helper methods affects many tests.
>
>
> On 11/9/18 11:31 AM, Kirk Lund wrote:
>
>> *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. I'll explain why below.
>>
>> Here's an example for an IntegrationTest that needs a Cache but not any
>> CacheServers:
>>
>> private Cache cache;
>>
>> @Before
>> public void setUp() {
>>Properties config = new Properties();
>>config.setProperty(LOCATORS, "");
>>cache = new CacheFactory(config).create();
>> }
>>
>> @After
>> public void tearDown() {
>>cache.close();
>> }
>>
>> That's some pretty simple code and as a Developer, I can tell exactly what
>> it's doing and what the config is.
>>
>> Here's an example of the kind of Geode User API code that I use to create
>> Servers in a DistributedTests now:
>>
>>private void createServer(String serverName, File serverDir, int
>> locatorPort) {
>> ServerLauncher.Builder builder = new ServerLauncher.Builder();
>> builder.setMemberName(serverName);
>> builder.setWorkingDirectory(serverDir.getAbsolutePath());
>> builder.setServerPort(0);
>> builder.set(LOCATORS, "localHost[" + locatorPort + "]");
>> builder.set(DISABLE_AUTO_RECONNECT, "false");
>> builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
>> builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
>> builder.set(MEMBER_TIMEOUT, "2000");
>>
>> serverLauncher = builder.build();
>> serverLauncher.start();
>> assertThat(serverLauncher.isRunning()).isTrue();
>>}
>>
>> In particular, I think we should be using ServerLauncher and
>> LocatorLauncher instead of Rules when we want a full-stack Locator or
>> full-stack Server that looks like what a User is going to startup.
>>
>> Here are my reasons:
>>
>> 1) I want to learn and use the Geode User APIs directly, not someone's
>> (even mine) Testing API that hides the Geode User APIs. If I see a test
>> fail, I want to see exactly what was configured and what User APIs were
>> used right there in the test without having to open other classes. I don't
>> want to have to spend even 15 minutes digging through some JUnit Rule to
>> figure out how PDX was configured.
>>
>> 2) We need to make sure that the Geode User APIs are easy to use and are
>> complete. If we're writing tests against Testing APIs instead then we
>> don't
>> feel the Users' pain if our API is painful. If the reason to use a Rule is
>> because our User API is overly-verbose of difficult, then that's even more
>> reason to use the Geode User API, so we recognize that it needs to change!
>>
>> GemFire had a long history of hiding its User APIs behind elaborate
>> Testing
>> APIs and we all used these fancy, easier to use, more compact Testing
>> APIs.
>> This promotes complicated, inconsistent and potentially incomplete User
>> APIs for Users to actually use. The result: difficult to use product with
>> difficult to use APIs and User APIs that are missing important things that
>> then Users have to resort to internal APIs to use. I'm strongly convinced
>> that using elaborate Testing APIs is at least largely responsible for
>> making GemFire and now Geode difficult to use and that's why I'm pushing
>> so
>> hard to write tests with Geode User APIs instead of convenient custom
>> Rules
>>
>> Since I started using ServerLauncher and LocatorLauncher APIs directly in
>> my DistributedTests I made a very important discovery: the User has no way
>> to get a reference to the Cache. This is why I recently started a
>> discussion thread about add getCache and getLocator to these Launcher
>> APIs.
>> If we keep using elaborate Testing APIs including custom Geode JUnit Rules
>> to hide these APIs, we'll never make these discoveries that I feel are
>> vital for our Users. We need to make things a LOT easier for the Users
>> going forward.
>>
>> The above is why I think we should be using User APIs in the tests even
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 and are sometimes in
Helper classes. I've found these especially difficult to deal with when
fixing flaky tests because changing one of the Helper methods affects
many tests.
On 11/9/18 11:31 AM, Kirk Lund wrote:
*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. I'll explain why below.
Here's an example for an IntegrationTest that needs a Cache but not any
CacheServers:
private Cache cache;
@Before
public void setUp() {
Properties config = new Properties();
config.setProperty(LOCATORS, "");
cache = new CacheFactory(config).create();
}
@After
public void tearDown() {
cache.close();
}
That's some pretty simple code and as a Developer, I can tell exactly what
it's doing and what the config is.
Here's an example of the kind of Geode User API code that I use to create
Servers in a DistributedTests now:
private void createServer(String serverName, File serverDir, int
locatorPort) {
ServerLauncher.Builder builder = new ServerLauncher.Builder();
builder.setMemberName(serverName);
builder.setWorkingDirectory(serverDir.getAbsolutePath());
builder.setServerPort(0);
builder.set(LOCATORS, "localHost[" + locatorPort + "]");
builder.set(DISABLE_AUTO_RECONNECT, "false");
builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
builder.set(MEMBER_TIMEOUT, "2000");
serverLauncher = builder.build();
serverLauncher.start();
assertThat(serverLauncher.isRunning()).isTrue();
}
In particular, I think we should be using ServerLauncher and
LocatorLauncher instead of Rules when we want a full-stack Locator or
full-stack Server that looks like what a User is going to startup.
Here are my reasons:
1) I want to learn and use the Geode User APIs directly, not someone's
(even mine) Testing API that hides the Geode User APIs. If I see a test
fail, I want to see exactly what was configured and what User APIs were
used right there in the test without having to open other classes. I don't
want to have to spend even 15 minutes digging through some JUnit Rule to
figure out how PDX was configured.
2) We need to make sure that the Geode User APIs are easy to use and are
complete. If we're writing tests against Testing APIs instead then we don't
feel the Users' pain if our API is painful. If the reason to use a Rule is
because our User API is overly-verbose of difficult, then that's even more
reason to use the Geode User API, so we recognize that it needs to change!
GemFire had a long history of hiding its User APIs behind elaborate Testing
APIs and we all used these fancy, easier to use, more compact Testing APIs.
This promotes complicated, inconsistent and potentially incomplete User
APIs for Users to actually use. The result: difficult to use product with
difficult to use APIs and User APIs that are missing important things that
then Users have to resort to internal APIs to use. I'm strongly convinced
that using elaborate Testing APIs is at least largely responsible for
making GemFire and now Geode difficult to use and that's why I'm pushing so
hard to write tests with Geode User APIs instead of convenient custom Rules
Since I started using ServerLauncher and LocatorLauncher APIs directly in
my DistributedTests I made a very important discovery: the User has no way
to get a reference to the Cache. This is why I recently started a
discussion thread about add getCache and getLocator to these Launcher APIs.
If we keep using elaborate Testing APIs including custom Geode JUnit Rules
to hide these APIs, we'll never make these discoveries that I feel are
vital for our Users. We need to make things a LOT easier for the Users
going forward.
The above is why I think we should be using User APIs in the tests even for
setUp and tearDown. Save the custom JUnit Rules for NON-GEODE things like
configuring JSON or LOG4J or allowing use of ErrorCollector in all DUnit
VMs.
Thanks,
Kirk
On Fri, Nov 9, 2018 at 10:49 AM, Galen O'Sullivan
wrote:
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 inheritance, I think we should deprecate JUnit4CacheTestCase and
change the comment to imply that classes should inherit from it just
because they
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
*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. I'll explain why below.
Here's an example for an IntegrationTest that needs a Cache but not any
CacheServers:
private Cache cache;
@Before
public void setUp() {
Properties config = new Properties();
config.setProperty(LOCATORS, "");
cache = new CacheFactory(config).create();
}
@After
public void tearDown() {
cache.close();
}
That's some pretty simple code and as a Developer, I can tell exactly what
it's doing and what the config is.
Here's an example of the kind of Geode User API code that I use to create
Servers in a DistributedTests now:
private void createServer(String serverName, File serverDir, int
locatorPort) {
ServerLauncher.Builder builder = new ServerLauncher.Builder();
builder.setMemberName(serverName);
builder.setWorkingDirectory(serverDir.getAbsolutePath());
builder.setServerPort(0);
builder.set(LOCATORS, "localHost[" + locatorPort + "]");
builder.set(DISABLE_AUTO_RECONNECT, "false");
builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
builder.set(MEMBER_TIMEOUT, "2000");
serverLauncher = builder.build();
serverLauncher.start();
assertThat(serverLauncher.isRunning()).isTrue();
}
In particular, I think we should be using ServerLauncher and
LocatorLauncher instead of Rules when we want a full-stack Locator or
full-stack Server that looks like what a User is going to startup.
Here are my reasons:
1) I want to learn and use the Geode User APIs directly, not someone's
(even mine) Testing API that hides the Geode User APIs. If I see a test
fail, I want to see exactly what was configured and what User APIs were
used right there in the test without having to open other classes. I don't
want to have to spend even 15 minutes digging through some JUnit Rule to
figure out how PDX was configured.
2) We need to make sure that the Geode User APIs are easy to use and are
complete. If we're writing tests against Testing APIs instead then we don't
feel the Users' pain if our API is painful. If the reason to use a Rule is
because our User API is overly-verbose of difficult, then that's even more
reason to use the Geode User API, so we recognize that it needs to change!
GemFire had a long history of hiding its User APIs behind elaborate Testing
APIs and we all used these fancy, easier to use, more compact Testing APIs.
This promotes complicated, inconsistent and potentially incomplete User
APIs for Users to actually use. The result: difficult to use product with
difficult to use APIs and User APIs that are missing important things that
then Users have to resort to internal APIs to use. I'm strongly convinced
that using elaborate Testing APIs is at least largely responsible for
making GemFire and now Geode difficult to use and that's why I'm pushing so
hard to write tests with Geode User APIs instead of convenient custom Rules
Since I started using ServerLauncher and LocatorLauncher APIs directly in
my DistributedTests I made a very important discovery: the User has no way
to get a reference to the Cache. This is why I recently started a
discussion thread about add getCache and getLocator to these Launcher APIs.
If we keep using elaborate Testing APIs including custom Geode JUnit Rules
to hide these APIs, we'll never make these discoveries that I feel are
vital for our Users. We need to make things a LOT easier for the Users
going forward.
The above is why I think we should be using User APIs in the tests even for
setUp and tearDown. Save the custom JUnit Rules for NON-GEODE things like
configuring JSON or LOG4J or allowing use of ErrorCollector in all DUnit
VMs.
Thanks,
Kirk
On Fri, Nov 9, 2018 at 10:49 AM, Galen O'Sullivan
wrote:
> 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 inheritance, I think we should deprecate JUnit4CacheTestCase and
> change the comment to imply that classes should inherit from it just
> because they require a Cache.
>
> Is is worth deprecating JUnit4DistributedTestCase as well and encouraging
> the use of ClusterStartupRule instead?
>
> Thanks,
> Galen
>
Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
+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 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 inheritance, I think we should deprecate JUnit4CacheTestCase and > change the comment to imply that classes should inherit from it just > because they require a Cache. > > Is is worth deprecating JUnit4DistributedTestCase as well and encouraging > the use of ClusterStartupRule instead? > > Thanks, > Galen > -- Cheers Jinmei
[DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
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 inheritance, I think we should deprecate JUnit4CacheTestCase and change the comment to imply that classes should inherit from it just because they require a Cache. Is is worth deprecating JUnit4DistributedTestCase as well and encouraging the use of ClusterStartupRule instead? Thanks, Galen
