Thanks Daniël, I'll write up a more formal proposal / jira in the upcoming days
Andrew On Tue, Apr 13, 2021 at 11:37 AM Daniël Heres <danielhe...@gmail.com> wrote: > Late reply, but I agree these tests modules need a bit of reorganization. I > also found myself adding tests to context.rs / sql.rs just because > related/similar tests are included there. > > Sounds like a good reorganization to me! > > On Fri, Apr 9, 2021, 20:44 Andrew Lamb <al...@influxdata.com> wrote: > > > As Jorge points out here [1], the tests in datafusion/src/context.rs are > > not really unit tests. They are more like SQL integration tests. There is > > also a small and languishing set of sql tests in `rust/datafusion/tests/ > > sql.rs`. > > > > These tests are critical for DataFusion's quality and I would like to > > propose a small reorganization so it is easier to find existing test > > coverage and write new ones > > > > Specifically I propose: > > 1. move `rust/datafusion/src/test` to its own module `rust/test_helpers` > > (so that it can be shared with sql.rs) > > 2. Update the style of all sql.rs tests to be inline with that in > > context.rs > > (using assert_batches_eq!) > > 3. Move tests that are not specific to `ExecutionContext` out of > > context.rs > > and into sql.rs > > > > Then over time I imagine being able to organize the tests within sql.rs > > better (split into multiple modules, for example) > > > > If no one objects, I'll write up some JIRA tickets and start trying to > move > > in this direction > > > > Thanks, > > Andrew > > > > [1] > https://github.com/apache/arrow/pull/9936#pullrequestreview-632020250 > > >
