ExUnit: async exclusion groups instead of async off

[Paul Dann]

I always find it a real shame when I have to turn async off for an ExUnit test file due to some global state that's shared with a test in only one or two other test files. What I _really_ want to do is to specify that _these_ tests shouldn't be run at the same time as _those_ tests. The whole of the rest of the test suite can be run simultaneously without issue.

Could we solve the problem by defining exclusion groups, for instance:

use ExUnit.Case, async: true, async_exclude: [:fakes_a_global_genserver]

The test file is then guaranteed not to run simultaneously with any files that provide any of the same atoms in the async_exclude list.

Paul

[José Valim]

Why are tests that fake a global gen server running asynchronously? :) If they change global state, how can you make sure in the long term, they are not going to affect any other test in the system?

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/f6d66e30-e15c-4de3-a48c-321791a51de4n%40googlegroups.com.

[Paul Dann]

On Fri, 26 Nov 2021 at 13:12, José Valim <jose....@dashbit.co> wrote:

Why are tests that fake a global gen server running asynchronously? :) If they change global state, how can you make sure in the long term, they are not going to affect any other test in the system?

Well, I think the same question could be asked of any sync test file. A test will only really need to run sync if it affects global state in some way. An example from my current project is a NoSQL database that doesn't support transactions. Most test files only work with a single table, but if two test files use the same table, then I need those particular tests not to run simultaneously. They can run at the same time as all the _other_ tests just fine, though - many of those don't even touch the database.

It seems like a generally sensible rule that tests that need to run sync also need to ensure that they restore or reset whatever global state it was that required them to run sync. For instance, if my test file has a setup function that calls Application.put_env() to change some global config (such as switching which genserver is used for a given task to a fake to support the test), then the config should be switched back again afterwards, or tests run after that point will fail. I think that's as true of the current situation with "async: off" as it would be with exclusion groups :)

Paul

[José Valim]

Right, my concern is not about the existing tests, which indeed need to revert, but future changes. Taking the application environment example, now anyone in the future that adds a feature that reads from that environment or writes a test that reads from that environment, they need to remember to annotate their new test case to exclude a completely unrelated file, that may have been written 1 week, 1 month, or 1 year ago.

--

You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CALZj-Vq5eJPCkoXYLhAn6O_vXTnLDO%2BhhwCSJT8JN4e7rTZhEw%40mail.gmail.com.

[Paul Dann]

On Fri, 26 Nov 2021 at 14:24, José Valim <jose....@dashbit.co> wrote:

Right, my concern is not about the existing tests, which indeed need to revert, but future changes. Taking the application environment example, now anyone in the future that adds a feature that reads from that environment or writes a test that reads from that environment, they need to remember to annotate their new test case to exclude a completely unrelated file, that may have been written 1 week, 1 month, or 1 year ago.

I think you're saying that in the Application.put_env() case, the safe option is to always mark a test file async: false as soon as global state such as this is used, and then future tests are isolated from this test as well. And that's true; that probably remains the safest approach. But maybe in that situation, when you're about to switch async: true to false in your new test file, a second alternative is to check for existing uses of the same global state in the test suite, and adding an exclusion group instead.

If code is refactored to use affected global state, it shouldn't take too long for the race conditions to start appearing in the CI, at which point in my experience it's not too difficult to track down where global state is used in that test file. I think that can pretty easily happen to regular async tests too. The time saved in test suite runs can outweigh the pain of diagnosing a false-failure, I think, depending on the project. I'm certainly not saying this is the best way for all projects - it requires care, much like running Ecto in async tests requires care and a good understanding of the problem.

Paul

[Adam Lancaster]

I think the point is that an exclusion group needs to be managed manually, which means if you add a new module you have to check every test to see if the module needs adding to an exclusion group there... which seems worse than just making the test async: false.

Worth pointing out (you may or may not know) you can pull your synchronous tests into their own module so the absolute minimum number of tests are running async which seems to be what you are after? Also that module can be in the same test file as a test module with async tests inside them.

Best

Adam

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CALZj-Vp2YZRooJtB2oCd4HkHwHFBL9kXb6VapZFRPGV8j89XBw%40mail.gmail.com.

[José Valim]

Yes, thank you Paul and Adam for "translating" my intent. :)

> I'm certainly not saying this is the best way for all projects - it requires care, much like running Ecto in async tests requires care and a good understanding of the problem.

I don't agree with the comparison because Ecto tests are sandboxed. Sure, there are consequences of using the sandbox, but it is a much safer by default approach than the approach proposed here.

To be clear, I understand and agree with the problem, but I don't agree with the solution because it is not ultimately solving the problem at hand. For example, speaking about Ecto, you could also use Mox, which also has an ownership-like mechanism, similar to Ecto's. You could define a behaviour, provide a default value for said behaviour, and then mock it in specific tests. This means your tests can run concurrently all the time. However, that sounds like overengineering for something as simple as reading the application environment. In any case, I hope it provides another frame of reference.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/0E0554D5-3C7D-4650-B77B-53B2A158DC26%40a-corp.co.uk.

[Paul Dann]

Sorry for the delay in response!

On Fri, 26 Nov 2021 at 16:49, Adam Lancaster <ad...@a-corp.co.uk> wrote:

I think the point is that an exclusion group needs to be managed manually, which means if you add a new module you have to check every test to see if the module needs adding to an exclusion group there... which seems worse than just making the test async: false.

Well, only if you want the new module to run async and are willing to put in a little extra effort to achieve that. There would of course still be the simple option of using async: false. I'd expect that a good naming scheme would help. The situation is not _too_ dissimilar from test tags in that regard - they need to be manually checked for consistency or a test may be incorrectly excluded from CI, for instance.

 

Worth pointing out (you may or may not know) you can pull your synchronous tests into their own module so the absolute minimum number of tests are running async which seems to be what you are after? Also that module can be in the same test file as a test module with async tests inside them.

A good reminder; thanks. But many of the tests I'm working with at the moment deal with temporary fixture data in a db that doesn't support transactions. Most test files use different tables, though, so running async is generally fine (and desirable, because some tests are long-running), except for one or two that shouldn't be run at the same time as specific other test files because of touching the same tables.

 

On Fri, 26 Nov 2021 at 20:22, José Valim <jose....@dashbit.co> wrote:

To be clear, I understand and agree with the problem, but I don't agree with the solution because it is not ultimately solving the problem at hand. For example, speaking about Ecto, you could also use Mox, which also has an ownership-like mechanism, similar to Ecto's. You could define a behaviour, provide a default value for said behaviour, and then mock it in specific tests. This means your tests can run concurrently all the time. However, that sounds like overengineering for something as simple as reading the application environment. In any case, I hope it provides another frame of reference.

Quite right - I do in fact rely on fakes quite extensively to support tests, but many of the tests I'm considering are intended to test database queries, so I can't really fake them out. I honestly haven't yet looked in detail at Mox, so if it has some kind of checkout mechanism that could act as a semaphore, I suppose that could be a possible path to a solution (maybe a bit heavy), but as I said I'm not really looking to mock the global state, just serialise tests in groups according to the global resources they touch.

Paul

[Christopher Keele]

Not weighing in on the intent here, but the mechanism:

It probably makes sense to leverage the existing tagging mechanism here. We could then just configure ExUnit like:

use ExUnit.Case

[Christopher Keele]

sorry, early posted.

Not weighing in on the intent here, but the mechanism:

It probably makes sense to leverage the existing tagging/filtering mechanism here. We could then just configure ExUnit like:

use ExUnit.Case, independent: [:db_case1, :db_case2]

Then treat non-flagged tests as one run, then each independent case as another...

But then using tagging, without adding to ExUnit, you could just use filters at the CLI level and run separate suites in sequence at the CI level. ex:

mix test --exclude db_case1:true --exclude db_case2:true mix test --include db_case1:true mix test --include db_case2:true

Would this approach solve what you are trying to do?

[Paul Dann]

On Tue, 30 Nov 2021 at 16:47, Christopher Keele <christ...@gmail.com> wrote:

use ExUnit.Case, independent: [:db_case1, :db_case2]

mix test --exclude db_case1:true --exclude db_case2:true mix test --include db_case1:true mix test --include db_case2:true

Thanks for weighing in on the mechanism :) If I understand your approach here, I think all the test files in the `db_case1` group will be run simultaneously and separately from the bulk of the tests, which is unfortunately the reverse of the effect I'm looking for. What I'd like to achieve is for each test file in `db_case1` to run async at the same time as the bulk of the tests, but only one file at a time that is tagged `db_case1`. So in the example above, it's "async" with respect to all test files _except_ other test files in the `db_case1` and `db_case2` groups. It is effectively "async: false" with respect only to other files in those groups. I can't see an easy way to handle that directly with tag inclusion / exclusion, but I imagine some of the internal tagging mechanism could be helpful in implementation?

Paul

[Paul Dann]

On Tue, 30 Nov 2021 at 12:27, Paul Dann <pdgi...@gmail.com> wrote:

On Fri, 26 Nov 2021 at 20:22, José Valim <jose....@dashbit.co> wrote:

To be clear, I understand and agree with the problem, but I don't agree with the solution because it is not ultimately solving the problem at hand. For example, speaking about Ecto, you could also use Mox, which also has an ownership-like mechanism, similar to Ecto's. You could define a behaviour, provide a default value for said behaviour, and then mock it in specific tests. This means your tests can run concurrently all the time. However, that sounds like overengineering for something as simple as reading the application environment. In any case, I hope it provides another frame of reference.

Quite right - I do in fact rely on fakes quite extensively to support tests, but many of the tests I'm considering are intended to test database queries, so I can't really fake them out. I honestly haven't yet looked in detail at Mox, so if it has some kind of checkout mechanism that could act as a semaphore, I suppose that could be a possible path to a solution (maybe a bit heavy), but as I said I'm not really looking to mock the global state, just serialise tests in groups according to the global resources they touch.

I spent some time recently trying to solve this problem by looking into whether I can scope database access to specific tests. Inspired by Mox, I looked into using $callers to track pids. The problem I have is that the data store I'm using (ElasticSearch) does not have transaction support. I'm experimenting with scoping the actual _name_ of indexes (tables) used for each test, but indications so far are that it's unlikely this could work transparently, which leads me back to a situation where tests need to be explicitly tagged in some way as accessing a particular shared storage in order to set up the namespacing required to prevent collisions. This is exactly the same kind of tag curation that exclusion groups would require, and probably actually introduces more complexity.

Ultimately, maybe I should just give up on async tests for this project, but it seems like a viable solution is frustratingly close. I agree that exclusion groups would require care to prevent race conditions, but I'm not seeing a good alternative when the database itself doesn't have transaction support, and can't be mocked due to the queries themselves being under test.

Paul

[José Valim]

I believe the constraints have not changed on our side. Explicitly saying "don't run alongside those files" feels a brittle way of declaring the dependencies between tests. Something like "async: :group_name" would work better, and that would say "it runs asynchronously but only one within said group name". So overall we have:

  * if true, runs the tests asynchronously with other modules

  * if false, runs the tests synchronously with other modules

  * if an atom, runs the tests synchronously with modules in the same group (atom) and asynchronously with the remaining ones

The big question is: would we want the opposite? If an atom, runs the tests asynchronously with modules in the same group and synchronously with the remaining ones? And I would say that sounds doable too. So the next challenge is coming up with a descriptive enough API that supports these scenarios.

One option could be: "async: true | false | {:async_within, :group} | {:async_outside, :group}", but I am not pleased about the async async_within and async_outside names. We don't need to support all cases upfront either, but we should consider the API.

--

You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CALZj-VpAEpeLGTD-de2nW6Gyyxew%2Bk%3De1GDJKWEkOMd9PKeXcA%40mail.gmail.com.

[Jon Rowe]

How about `async: [with: :group]` as "run asynchronously with other tests with this group name" and `async: [except: :group]` as "run synchronously with this group"

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4KJpPPucGKJYffMxBT82nd5F4x640rEzwK86sokrR-Zgw%40mail.gmail.com.

[José Valim]

I considered those options but I don't think they are intention revealing enough. I feel like I would have to always consult the docs to be sure which one is which. I would also go with a tuple, since those options do not really combine.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/6d006e1a-f653-482e-9704-607f4cda951e%40www.fastmail.com.

[José Valim]

To be clear, I think my initial suggestions are bad too. Especially async_outside. :)

[Christopher Keele]

> async: true | false | {:async_within, :group} | {:async_outside, :group}

Best ideas I can come up with are async: {isolate: :group}, and async: {alongside: :group}, but not sure how much better that is.

[Paul Dann]

On Wed, 5 Jan 2022 at 10:12, José Valim <jose....@dashbit.co> wrote:

  * if true, runs the tests asynchronously with other modules

  * if false, runs the tests synchronously with other modules

  * if an atom, runs the tests synchronously with modules in the same group (atom) and asynchronously with the remaining ones

Perfect! This is exactly what I'm proposing :)

 

The big question is: would we want the opposite? If an atom, runs the tests asynchronously with modules in the same group and synchronously with the remaining ones? And I would say that sounds doable too.

I guess if it's easy to do at the same time, but I think the same effect could be achieved using tags: multiple runs of `mix test --only group_<x>` would separate the groups, running tests within each async, but each group synchronously with respect to the next.

 

> async: true | false | {:async_within, :group} | {:async_outside, :group}

Best ideas I can come up with are async: {isolate: :group}, and async: {alongside: :group}, but not sure how much better that is.

I'd suggest matching the naming convention for filtering tags from mix:

async: {:exclude, [:group1, :group2]}

async: {:only, [:group1, :group2]}  # if this feature is wanted

I do think we need a list, as there may be multiple groups we need to exclude from running simultaneously. I am also a little concerned about confusion between groups defined here for async groups, and test module tags.

Paul