Arina, Thanks for the response. See responses inline: > On Jan 3, 2020, at 4:40 AM, Arina Yelchiyeva <[email protected]> > wrote: > > Hi Charles, > > I would expect contributor to provide efficient way to test storage plugins. > Depending on the source, some have embedded versions, some don’t. So > contributor should provide instructions how plugin can be tested. > Ideally, tests should be able to run without dependency on external system. > Kudu is old plugin and does not comply with Drill coding standards, so the > fact it does not have unit tests does not mean that other won’t have.
I wasn't suggesting that new plugins shouldn't have good testing ;-). Far from it. What I am wondering is whether it is necessary to have tests for every class, or can we go for good coverage by providing a lot of query unit tests that test a lot of different cases? Let's say I'm writing a plugin for Druid and Druid doesn't have an embedded mode. (I have no idea whether Druid does or doesn't, just using it as an example.) Would it be acceptable for a developer to provide a Docker container loaded with Druid and a small dataset to serve as a testbed for the unit tests? Just FYI, for the API plugin, I wrote 16 or so unit tests using a mock webserver, but that one was relatively easy because there are no shortage of libraries for testing HTTP responses. > > I would suggest we won’t accept storage plugins that does not have good unit > tests coverage, documentations and code quality. Since plugins are easily > pluggable, if plugin does not qualify Drill coding standards, it can be build > manually and added as jar to the Drill classpath. I completely agree. One of my biggest frustrations when writing the Drill book was the amount of undocumented functionality in Drill. One thing however, is that I don't think plugins are truly "pluggable" yet because most if not all require updates to the protobufs. > From my perspective, Drill users (or any users) expect if code is available > from official build, it means it fully tested and works. Otherwise, users > would complain that somethings does not work and say the Drill product is of > bad quality. > > Regarding code standards, this should apply to all PRs. We don’t have many > code reviewers for the project and submitting PRs that have poor code quality > would mean that these people (who voluntarily spend time reviewing and > understanding someone code) would have to spend lots of time reviewing and > correcting simple things. Many Apache projects have high code quality > standards, much higher than Drill, so I think it would be fair if contributor > would spend more time making code better than expect reviewer to point to > every trivial issue. > > Kind regards, > Arina > >> On Jan 2, 2020, at 8:50 PM, Charles Givre <[email protected]> wrote: >> >> Hello Drill Devs, >> I wanted to ask a question about testing storage plugins. Currently there >> are PRs for storage plugins for Apache Druid >> (https://github.com/apache/drill/pull/1888 >> <https://github.com/apache/drill/pull/1888>), and a generic REST API plugin >> (https://github.com/apache/drill/pull/1892 >> <https://github.com/apache/drill/pull/1892>). >> >> I wanted to ask about what is necessary with respect to testing to get a >> storage plugin accepted? I looked at some of the others, and some, like the >> HBase plugin, have many unit tests and others, like the Kudu plugin barely >> have any. Also, since obviously, these unit tests require an external >> system (or at least a docker container of one) what should a contributor >> provide with a storage plugin? Should they provide a docker container with >> a data set pre-loaded, or should the unit tests be marked "Ignore"? >> >> When we do releases, are we running the tests against external systems? >> Thanks, >> -- C >> >> >> >> >
