I like the "Complete Vertical Slide" recommendation. It goes against the wisdom of having focused unit tests, but I think in my experience, the tests that shake out the most bugs (and are most robust to refactoring) have been ones that wrap together a lot of layers.
One thing I didn't see in the article, but also like, is declarative tests (like the ones in CalciteQueryTest). I think they're short, effective at catching bugs, easy to read, and easy to write more of. They would be even better if they used builder-style methods instead of positional arguments. I don't think the "Don’t Use In-Memory Databases" recommendation applies to us. The idea is that you should test with something similar to production. But Druid is super-pluggable, and it supports a ton of options for databases, deep storages, and other integrations in production. We use Derby for metadata and local fs for deep storage in most tests, which _are_ supported production options. It's just that there are _also other_ options that we are not testing in unit tests. This has caused regressions in the past and I suppose calls for a test matrix. In the long run, we might benefit from bringing some of this stuff "in house". Imagine metadata stores were no longer pluggable, but instead there was only one supported option that lived on the Coordinator. It would be an effort to develop that, but then testing becomes simpler since one dimension of the matrix goes away. On Fri, Oct 4, 2019 at 9:44 AM Roman Leventov <leven...@apache.org> wrote: > A few days ago I've stumbled upon a blog post "Modern Best Practices for > Testing in Java" ( > https://phauer.com/2019/modern-best-practices-testing-java/) and learned a > lot of new things from it. > > In a lot of cases, how testing is done in Druid goes against these best > practices: > - "Given, When, Then" structure: in Druid, rarely followed or clarified > via blank lines. > - Use the Prefixes “actual*” and “expected*: not in Druid, usually it's > something1 / something2 and what is expected and what is actual is unclear > (and sometimes confused in assert() args) > - Heavily Use Helper Functions: used sometimes in Druid, but by far not > enough, in my perception > - Don’t Extend Existing Tests To “Just Test One More Tiny Thing: not > everywhere, but in a lot of places this practice is abused in Druid tests > - Insert Test Data Right In The Test Method: test data is often inserted > in @Before in Druid > - Favor Composition Over Inheritance: there are some hierarchies in Druid > - Don’t Write Too Much Logic: Druid tests feel like *all* > boilerplate/noise/scaffolding logic > - Don’t Use In-Memory Databases For Tests: Druid does toy database Derby > for tests :) > - Use AssertJ: Druid doesn't use AssertJ (I've just added AssertJ to > dependencies in https://github.com/apache/incubator-druid/pull/8564 in one > module) > - Use JUnit5: ¯\_(ツ)_/¯ > - Mock Remote Service: in Druid, we don't use OkHttp's MockWebServer > - Use Awaitility for Asserting Asynchronous Code: in Druid, we use ad-hoc > while loops with sleep(10), sleep(100), etc. > - Separate Asynchronous Execution and Actual Logic: I think, this is one > of the most important points. In Druid, in almost all cases, execution > management (thread pools), dependency injection (and dependencies in > general), and the "core" logic are alloyed in a single class. This makes > testing the core logic very painful with a lot of mocking and setup > ceremony, breaking frequently. Following the Humble Object pattern ( > http://xunitpatterns.com/Humble%20Object.html) would make writing *and > supporting* much less laborious. > > These things cannot be fixed in one day because test code has a lot of > inertia, but I believe to improve developer productivity all active Druid > developers should start using most of these practices in all new tests > being written, demand other contributors align their tests with these > practices in code reviews, and take some time to think about strategic ways > to refactor a bulk of existing tests with little effort > (semi-automatically) that can at the same time improve their quality even > by a little. > > In my experience, wrestling with tests is a huge (often the biggest) part > of changing existing code in Druid, so we pay a huge productivity tax by > not paying enough attention to the quality of the test codebase. >
