Add section on testing to the Developers doc

[alamb]

Which issue does this PR close?

re #743 and #1453

See rendered https://github.com/alamb/arrow-datafusion/blob/alamb%2Ftesting_docs/DEVELOPERS.md

Rationale for this change

Start to document the current (and maybe aspirational) test structure so we know what, if anything, should be changed as we "clean things up"

What changes are included in this PR?

A section on Testing in the DEVELOPERS doc

Are there any user-facing changes?

no

cc @hntd187 @xudong963 @jimexist @houqp

[alamb]

One thing I was thinking @hntd187 and @matthewmturner would be to put some guidance here like "tests that run a full sql query likely belong in sql.rs" (also perhaps describe which tests should go in the "SQL / Postgres integration tests" and which are good to be in sql.rs)

[xudong963]

Agree.

[hntd187]

I was going to grain it down a bit more with a module for sql but then specific modules separated by functionality such as "order_by", "group_by" things of that nature, where generally they fall under sql, but more specifically if they touch specific functionality they go deeper. I'm not sure what @matthewmturner has started so maybe I'll wait a day to see what he says.

I also wanted to separate tests for internal things but the unit tests inside individual code seem fine for that.

[matthewmturner]

@alamb @hntd187 sounds good. i have not had a chance to start on this yet and likely wont be able to spend much time on it until i finish some other PRs. @hntd187 it seems like you have a good plan and id be happy to follow your lead. assuming that breaking out the sql functionality like that is the agreed upon approach maybe we could make a parent issue with sub tasks for each functionality / internal thing and then i could pick up on that once i have some more time?

[hntd187]

Sure I've started moving things around, and I guess I'm wondering how deep down the rabbit hole do we wanna go here? There are a good number of tests in sql.rs that by their name cross considerations, like is reading parquet as a format something for sql? In addition a lot of tests are prepended with csv, which seems somewhat redundant to me, is there a purpose behind it? I think I will reorganize the tests themselves first, touch no code other than moving around and then we can talk about renaming or any actual code changes.

[alamb]

In addition a lot of tests are prepended with csv, which seems somewhat redundant to me, is there a purpose behind it?

Not that I can remember

I think I will reorganize the tests themselves first, touch no code other than moving around and then we can talk about renaming or any actual code changes.

I think that is an excellent strategy 👍

[xudong963]

The descriptions of our existing tests are clear in the ticket, thanks @alamb.

I opened an issue about sqllogictest #1453 the other day. The reason why I opened it is because I think the SQL / Postgres Integration Tests isn't solid, it doesn't have a test scale. (Indeed Datafusion mainly relies on sql.rs for testing).

If I understand correctly, the test refactoring that @hntd187 and @matthewmturner are doing doesn't conflict with what I mentioned above. They mainly want to better classify existing tests, which should be unit tests and which should be integration tests. I think it makes sense to me, thanks @hntd187 @matthewmturner. Now we're used to putting tests directly in sql.rs, but sometimes it's better to put it in a source file with a test mod.

Back to my sqllogictest, I think if we can improve the scale of the SQL / Postgres Integration Tests, sqllogictest mentioned by me is not necessary.

[alamb]

I was going through the PRs in datafusion and ran across this one. Given I am planning (hoping) we have more people interested in the project after the 7.0.0 release I wanted to get this documented a bit more

[alamb]

@jimexist is this ok instructions for invoking the integration test locally?

It would probably be nicer to make it a little easier (like put the table creation in sql and default some of the arguments) but perhaps we can do that as a follow on ticket