Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lib/ex_unit/lib/ex_unit/case.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ defmodule ExUnit.Case do
It should be enabled only if tests do not change any global state.
Defaults to `false`.

* `:group` - configures async tests in this module to run within a group.
Tests in the same group never run concurrently. Tests from different
groups can run concurrently with `async: true`.
Defaults to `nil`.

* `:register` - when `false`, does not register this module within
ExUnit server. This means the module won't run when ExUnit suite runs.

Expand Down Expand Up @@ -317,7 +322,7 @@ defmodule ExUnit.Case do
end

{register?, opts} = Keyword.pop(opts, :register, true)
{next_opts, opts} = Keyword.split(opts, [:async, :parameterize])
{next_opts, opts} = Keyword.split(opts, [:async, :group, :parameterize])

if opts != [] do
IO.warn("unknown options given to ExUnit.Case: #{inspect(opts)}")
Expand Down Expand Up @@ -552,6 +557,7 @@ defmodule ExUnit.Case do

opts = Module.get_attribute(module, :ex_unit_module, [])
async? = Keyword.get(opts, :async, false)
group = Keyword.get(opts, :group, nil)
parameterize = Keyword.get(opts, :parameterize, nil)

if not (parameterize == nil or (is_list(parameterize) and Enum.all?(parameterize, &is_map/1))) do
Expand All @@ -569,7 +575,11 @@ defmodule ExUnit.Case do
end

def __ex_unit__(:config) do
{unquote(async?), unquote(Macro.escape(parameterize))}
%{
async?: unquote(async?),
group: unquote(Macro.escape(group)),
parameterize: unquote(Macro.escape(parameterize))
}
end
end
end
Expand Down
15 changes: 13 additions & 2 deletions lib/ex_unit/lib/ex_unit/runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,22 @@ defmodule ExUnit.Runner do
running
end

defp spawn_modules(config, [{module, params} | modules], async?, running) do
defp spawn_modules(
config,
[{_group, group_modules} | modules],
Copy link
Member

@josevalim josevalim Oct 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sync modules are not arriving in this shape and they are being skipped (because the group modules is treated as an empty map). I will add a pattern to help us catch it. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, that masked the other issues!
Thanks for fixing it.

And bummer about the tests being flaky, it would have been nice to have tests for this functionality.

async?,
running
) do
if max_failures_reached?(config) do
running
else
{pid, ref} = spawn_monitor(fn -> run_module(config, module, async?, params) end)
{pid, ref} =
spawn_monitor(fn ->
Enum.each(group_modules, fn {module, params} ->
run_module(config, module, async?, params)
end)
end)

spawn_modules(config, modules, async?, Map.put(running, ref, pid))
end
end
Expand Down
80 changes: 68 additions & 12 deletions lib/ex_unit/lib/ex_unit/server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,21 @@ defmodule ExUnit.Server do
GenServer.start_link(__MODULE__, :ok, name: @name)
end

def add_module(name, {async?, parameterize}) do
def add_module(name, config) do
%{
async?: async?,
group: group,
parameterize: parameterize
} = config

modules =
if parameterize do
Enum.map(parameterize, &{name, &1})
else
[{name, %{}}]
end

case GenServer.call(@name, {:add, async?, modules}, @timeout) do
case GenServer.call(@name, {:add, {async?, group}, modules}, @timeout) do
:ok ->
:ok

Expand Down Expand Up @@ -51,6 +57,7 @@ defmodule ExUnit.Server do
state = %{
loaded: System.monotonic_time(),
waiting: nil,
async_groups: %{},
async_modules: :queue.new(),
sync_modules: :queue.new()
}
Expand All @@ -74,10 +81,20 @@ defmodule ExUnit.Server do

# Called by the runner when --repeat-until-failure is used.
def handle_call({:restore_modules, async_modules, sync_modules}, _from, state) do
{async_modules, async_groups} =
Enum.reduce(async_modules, {[], []}, fn
{nil, [module]}, {modules, groups} ->
{[{:module, module} | modules], groups}

{group, group_modules}, {modules, groups} ->
{[{:group, group} | modules], Map.put(groups, group, group_modules)}
end)

{:reply, :ok,
%{
state
| loaded: :done,
async_groups: async_groups,
async_modules: :queue.from_list(async_modules),
sync_modules: :queue.from_list(sync_modules)
}}
Expand All @@ -91,7 +108,15 @@ defmodule ExUnit.Server do
when is_integer(loaded) do
state =
if uniq? do
async_modules = :queue.to_list(state.async_modules) |> Enum.uniq() |> :queue.from_list()
async_modules =
state.async_modules
|> :queue.to_list()
|> Enum.uniq()
|> Enum.map(fn {group, modules} ->
{group, Enum.uniq(modules)}
end)
|> :queue.from_list()

sync_modules = :queue.to_list(state.sync_modules) |> Enum.uniq() |> :queue.from_list()

%{
Expand All @@ -107,26 +132,40 @@ defmodule ExUnit.Server do
{:reply, diff, take_modules(%{state | loaded: :done})}
end

def handle_call({:add, true, names}, _from, %{loaded: loaded} = state)
def handle_call({:add, {false = _async, _group}, names}, _from, %{loaded: loaded} = state)
when is_integer(loaded) do
state =
update_in(state.sync_modules, &Enum.reduce(names, &1, fn name, q -> :queue.in(name, q) end))

{:reply, :ok, state}
end

def handle_call({:add, {true = _async, nil = _group}, names}, _from, %{loaded: loaded} = state)
when is_integer(loaded) do
state =
update_in(
state.async_modules,
&Enum.reduce(names, &1, fn name, q -> :queue.in(name, q) end)
&Enum.reduce(names, &1, fn name, q -> :queue.in({:module, name}, q) end)
)

{:reply, :ok, take_modules(state)}
{:reply, :ok, state}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We lost a call to take_modules here.

end

def handle_call({:add, false, names}, _from, %{loaded: loaded} = state)
def handle_call({:add, {true = _async, group}, names}, _from, %{loaded: loaded} = state)
when is_integer(loaded) do
state =
update_in(state.sync_modules, &Enum.reduce(names, &1, fn name, q -> :queue.in(name, q) end))
if Map.has_key?(state.async_groups, group) do
update_in(state.async_groups[group], &(names ++ &1))
else
state
|> put_in([:async_groups, group], names)
|> update_in([:async_modules], &:queue.in({:group, group}, &1))
end

{:reply, :ok, state}
end

def handle_call({:add, _async?, _names}, _from, state),
def handle_call({:add, {_async?, _group}, _names}, _from, state),
do: {:reply, :already_running, state}

defp take_modules(%{waiting: nil} = state) do
Expand All @@ -145,9 +184,26 @@ defmodule ExUnit.Server do
state

true ->
{modules, async_modules} = take_until(count, state.async_modules)
GenServer.reply(from, modules)
%{state | async_modules: async_modules, waiting: nil}
{async_modules, remaining_modules} = take_until(count, state.async_modules)

{async_modules, remaining_groups} =
Enum.reduce(async_modules, {[], state.async_groups}, fn
{:module, module}, {collected_modules, async_groups} ->
{[{nil, [module]} | collected_modules], async_groups}

{:group, group}, {collected_modules, async_groups} ->
{group_modules, async_groups} = Map.pop!(async_groups, group)
{[{group, group_modules} | collected_modules], async_groups}
end)

GenServer.reply(from, Enum.reverse(async_modules))

%{
state
| async_groups: remaining_groups,
async_modules: remaining_modules,
waiting: nil
}
end
end

Expand Down
165 changes: 165 additions & 0 deletions lib/ex_unit/test/ex_unit_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,123 @@ defmodule ExUnitTest do
assert_receive {:tmp_dir, tmp_dir2} when tmp_dir1 != tmp_dir2
end

test "async tests run concurrently" do
Process.register(self(), :async_tests)

defmodule FirstAsyncTest do
use ExUnit.Case, async: true

test "first test" do
send(:async_tests, {:first_test, :started})
Process.sleep(10)
assert true
send(:async_tests, {:first_test, :finished})
end
end

defmodule SecondAsyncTest do
use ExUnit.Case, async: true

test "second test" do
send(:async_tests, {:second_test, :started})
Process.sleep(15)
assert true
send(:async_tests, {:second_test, :finished})
end
end

configure_and_reload_on_exit(max_cases: 2)

test_task =
Task.async(fn ->
capture_io(fn -> ExUnit.run() end)
end)

# Expected test distribution through time
#
# Time (ms): 0 10 20
# |-----|-----|
# CPU0: ( 1 )
# CPU1: ( 2 )
assert_receive({:first_test, :started}, 5)
assert_receive({:second_test, :started}, 5)

# make sure we don't leave the task running after the outer test finishes
Task.await(test_task)
end

test "async tests run concurrently respecting groups" do
Process.register(self(), :async_grouped_tests)

defmodule RedOneTest do
use ExUnit.Case, async: true, group: :red

test "red one test" do
send(:async_grouped_tests, {:red_one, :started})
Process.sleep(30)
assert true
send(:async_grouped_tests, {:red_one, :finished})
end
end

defmodule RedTwoTest do
use ExUnit.Case, async: true, group: :red

test "red two test" do
send(:async_grouped_tests, {:red_two, :started})
Process.sleep(10)
assert true
send(:async_grouped_tests, {:red_two, :finished})
end
end

defmodule BlueOneTest do
use ExUnit.Case, async: true, group: :blue

test "blue one test" do
send(:async_grouped_tests, {:blue_one, :started})
Process.sleep(10)
assert true
send(:async_grouped_tests, {:blue_one, :finished})
end
end

defmodule BlueTwoTest do
use ExUnit.Case, async: true, group: :blue

test "blue two test" do
send(:async_grouped_tests, {:blue_two, :started})
Process.sleep(10)
assert true
send(:async_grouped_tests, {:blue_two, :finished})
end
end

configure_and_reload_on_exit(max_cases: 4)

test_task =
Task.async(fn ->
capture_io(fn -> ExUnit.run() end)
end)

# Expected test distribution through time
#
# Time (ms): 0 10 20 30 40
# |-----|-----|-----|-----|
# CPU0: ( R1 )( R2 )
# CPU1: ( B1 )( B2 )
assert_receive({:red_one, :started}, 5)
assert_receive({:blue_one, :started}, 5)
refute_receive({:red_two, :started}, 25)
assert_received({:blue_one, :finished})
assert_received({:blue_two, :started})
assert_receive({:blue_two, :finished}, 15)
assert_receive({:red_two, :started}, 15)

# make sure we don't leave the task running after the outer test finishes
Task.await(test_task)
end

describe "after_suite/1" do
test "executes all callbacks set in reverse order" do
Process.register(self(), :after_suite_test_process)
Expand Down Expand Up @@ -1101,6 +1218,54 @@ defmodule ExUnitTest do
assert third =~ "ThirdTestFIFO"
end

test "async test groups are run in compile order (FIFO)" do
defmodule RedOneFIFO do
use ExUnit.Case, async: true, group: :red

test "red one test" do
assert true
end
end

defmodule BlueOneFIFO do
use ExUnit.Case, async: true, group: :blue

test "blue one test" do
assert true
end
end

defmodule RedTwoFIFO do
use ExUnit.Case, async: true, group: :red

test "red two test" do
assert true
end
end

defmodule BlueTwoFIFO do
use ExUnit.Case, async: true, group: :blue

test "blue two test" do
assert true
end
end

configure_and_reload_on_exit(trace: true)

output =
capture_io(fn ->
assert ExUnit.run() == %{total: 4, failures: 0, excluded: 0, skipped: 0}
end)

[_, first, second, third, fourth | _] = String.split(output, "\n\n")

assert first =~ "RedOneFIFO"
assert second =~ "RedTwoFIFO"
assert third =~ "BlueOneFIFO"
assert fourth =~ "BlueTwoFIFO"
end

test "can filter async tests" do
defmodule FirstTestAsyncTrue do
use ExUnit.Case, async: true
Expand Down