I think the suggested format could be useful when applicable, I would consider expanding "function under test" to "area under test" to accommodate tests that have a wider scope than a single method.
For cases where that format may not be applicable, perhaps that means it's a test where additional javadocs/comments explaining what the test does/expects is a good idea (that would be enough info in my view). Since I don't think the proposed format applies universally, I would prefer starting it off as a suggestion/best practice instead of as a hard requirement and seeing how that goes. I'm indifferent to the use of underscores. On Mon, Dec 23, 2019 at 3:19 PM Gian Merlino <g...@apache.org> wrote: > Suneet, > > Sometimes it's hard to understand how things would improve without an > example. Could you point to a test file that you think would be improved by > this change? Also, there are some test files that I would struggle to fit > into this framework. It seems best suited to simple single-method unit > tests. What would you suggest doing for these examples? > > - CalciteQueryTest: Tests the SQL layer. There's one function per SQL query > being tested, but there is no specific function under test, and the > expected result is too complex to include in a method name. > - GroupByQueryRunnerTest: Tests the groupBy query engine. Similar structure > and similar issue to CalciteQueryTest. > - SelectorFilterTest: Tests the selector filter. But no specific function > is being tested. The "assertFilterMatches" helper is used to create test > cases that say 'this filter should match these rows'. > > Other notes: > > - IMO, the underscores are ugly, but acceptable if they buy us something. > - I'm not sure that the "expectedResult" part of the name is going to work > well. It's often something complex (like an array, or object, or long > string) that would be awkward to include in a method name. And with JUnit > assertions, the expected result is right there in the method call anyway. > > Looking forward to hearing your thoughts. Thanks for thinking about how to > make our testing better. > > Gian > > On Mon, Dec 23, 2019 at 11:06 AM Suneet Saldanha <suneet.salda...@imply.io > > > wrote: > > > Hello, > > > > I've started writing tests in the druid repo and would like to propose a > > new test naming structure for the project. Inspired by this thread - > > https://www.mail-archive.com/dev@druid.apache.org/msg02426.html but > > focused > > only on the naming of tests. I'd like to propose we start using the > format > > below > > > > test_<functionUnderTest>_<conditions>_<expectedResult> > > > > This makes it a lot easier for devs to name tests in a way that's easily > > understood by someone else without having to read the test to know what's > > going on. Here's some of my rationale: > > > > - Explicit - the name tells you everything you need to know about the > > test > > - Forces you to write one test per condition/ expected result > > - Underscores make it easy to delineate the different components of > the > > test > > - Minimal effort to think of a short name that correctly captures > > everything about the test while still being different from all the > other > > tests > > > > Happy to hear feedback / concerns about this approach. > > > > Suneet > > >
