Should we use `async: true` on all generated tests?

[josevalim]

Now that we have the SQL sandbox, we can async: true all tests. The question though is that this only actually applies for Postgrex, so we may need a generator flag to control this behaviour.

[ericmj]

This may be a confusing default. Today you have to presumably learn about async tests and their pitfalls before enabling them. This change will make it more likely to experience the pitfalls before you have learned how to solve them or even know that the issue is caused by async tests. Combined with the nature of test race conditions where you may hit them a while after you originally created the bug can make this a pretty bad experience.

[stevedomin]

@josevalim didn't Ecto always ship with a SQL sandbox?

[mikemorris]

I'd lean towards async: true for all generated tests (with a flag for Postgrex-only currently), and rely on generated tests and documentation as a way of conveying the benefits of async tests and providing patterns for how to write them effectively.

It's much easier to increase adoption of functionality if it's a default, and IMHO this is good direction to push in for guiding users towards writing better tests.

Some tips on documentation language that may help explain async tests can be found in ava, a JavaScript framework that runs tests in parallel by default and describes them as "atomic" tests.

• https://github.com/avajs/ava#temp-files
• Note at bottom of https://github.com/avajs/ava#before--after-hooks on caution around before/after hooks

[mikemorris]

@stevedomin I'm assuming this is referring to https://hexdocs.pm/phoenix_ecto/Phoenix.Ecto.SQL.Sandbox.html, an implementation on top of https://hexdocs.pm/ecto/2.0.2/Ecto.Adapters.SQL.Sandbox.html

[stevedomin]

@mikemorris for some reason I assumed José was talking about something new in Ecto 3.0 :)

[josevalim]

Oops, accident. :)

[popo63301]

@ericmj How can we learn about those pitfalls?