Allow using cargo nextest for running tests

[alamb]

Which issue does this PR close?

Closes #13044

Rationale for this change

I want to use cargo nextest: https://nexte.st/ is a faster way to run tests

I want to run like this

cargo nextest run

But that errors as explained on #13044

What changes are included in this PR?

1. Stub out some more CLI arguments in sqllogictests test runner so it doesn't interfere with nextest

Are these changes tested?

Yes, by CI

I also ran it manually and I can now run the entire test suite (other than sqllogictest) in 30s keeping all cores busy

(venv) andrewlamb@Andrews-MacBook-Pro-2:~/Software/datafusion$ cargo nextest run
...
        PASS [   3.946s] datafusion-physical-expr-common binary_map::tests::test_string_overflow
        PASS [  27.468s] datafusion::fuzz fuzz_cases::equivalence::projection::ordering_satisfy_after_projection_random
------------
     Summary [  29.137s] 5349 tests run: 5349 passed, 11 skipped

Are there any user-facing changes?

No this is a developer change

[findepi]

why Ok?

[alamb]

Because if it returns an error, cargo nextest aborts immediately

What is happening I think is that nextest lists all possible commands first (by running cargo test ... --list --format terse) and then uses that output to determine what is subsequently run.

So if the --list part returns an error then none of the tests run

[alamb]

Added some comments in fd15523

[findepi]

❤️
a very good explanation

[jayzhan211]

👍

[findepi]

if cargo nextest is supposed to work, are we also going to have CI guarantee that?

[alamb]

if cargo nextest is supposed to work, are we also going to have CI guarantee that?

That is an excellent question

One way to do this would be to use nextest to run the tests, which might improve the CI speed (though we would have to pay for the time to build/install nextest itself) -- we actually do this in influxdb_iox and it works pretty well.

In general I assume this is a developer convenience for a small number of developers, if it breaks as part of a subsequent PR we can also always fix it again and it won't affect other PRs. If these assumptions turn out to be untrue, we should reasses.

Thus in my opinion adding adding a CI test would impose a burden that is not commensurate with the value that would be gained

[alamb]

Thanks @findepi and @jayzhan211 for the reviews

[findepi]

cargo test vs cargo nextest -- there is more to than just speed.

cargo nextest runs each test in separate process, providing awesome isolation.
Is it good for developers? it should, but it actually depends.
The tools i tried (RustRover, VS Code) are capable of running a test module, but they don't seem to understand nextest, so they run without process-per-test isolation, which kills productivity if tests actuall leverage isolation.
(@alamb do you or someone at influx have a solution to this?)

so

• if it's just supported but not guaranteed (this PR), i am fine with the change -- LGTM
• if we want to embrace nextest as the way to run tests -- i am probably fine to, but let's make sure we understand benefits and drawbacks

[alamb]

if it's just supported but not guaranteed (this PR), i am fine with the change -- LGTM

if we want to embrace nextest as the way to run tests -- i am probably fine to, but let's make sure we understand benefits and drawbacks

I don't think this is necessary / good to try and push. I think allowing developers the freedom to use whatever tools they deem best is better than trying to require one particular toolset

they run without process-per-test isolation, which kills productivity if tests actuall leverage isolation.
(@alamb do you or someone at influx have a solution to this?)

TLDR is we make sure all our tests are isolated / don't use global state -- which takes quite a bit of finagling when it involves subprocesses / end to end type tests.

[alamb]

Thank you again @findepi and @jayzhan211 for your helpful comments

[Omega359]

Would this be worthy of adding to https://github.com/apache/datafusion/blob/main/docs/source/contributor-guide/testing.md ?

[alamb]

Would this be worthy of adding to https://github.com/apache/datafusion/blob/main/docs/source/contributor-guide/testing.md ?

Yes, totally -- proposed change in #13160