Yes, we need a precommit test there and some documentation. A developer could end up adding tests that do not even conform to a more wide *test* and even if we skip abstract classes, only half the problem is taken care of. Instead, we should * document that *Test.class and Test*.class are acceptable test class names. * add a pre-commit check to make sure that tests are implemented in classes that confirm to the above pattern (at this point, I have no idea about how to do that but I can look it up). The developer shouldn't commit an abstract class that matches either of the above pattern as it would always lead to failed 'ant test', which I assume all of us run before we commit.
Thoughts? On Fri, Mar 4, 2016 at 6:53 PM, Erick Erickson <[email protected]> wrote: > Seems trappy to not allow BlahTests.class though. What about some kind of > precommit test? I can imagine some rules about naming of abstract classes > to avoid running them. But that's hacky too; if I name an abstract class > AbstractBlahTest, wouldn't it get run with the current rules? > > Erick > On Mar 5, 2016 14:15, "Shawn Heisey" <[email protected]> wrote: > >> On 3/4/2016 4:28 PM, Erick Erickson wrote: >> > >> > Oh my! Can't look now, but if I'm not missing something, I'd vote for >> > changing the pattern to include *Tests... >> > >> >> I discovered that there's an abstract class in SolrJ named >> "SolrExampleTests" that fails with the new pattern added. It fails >> because it's abstract. I did not check whether there were any classes >> elsewhere in the codebase that would fit this pattern -- I was playing >> around in SolrJ. >> >> Anshum did commit a pattern change, but after I discovered the above >> problem, opted to rename the test instead of changing the patterns. >> >> Thanks, >> Shawn >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >> -- Anshum Gupta
