Skip to content

Commit

Permalink
Clean up manifest implementation
Browse files Browse the repository at this point in the history
  * Use read instead of load_from to align with Elixir conventions

  * Only check for function_exported? after Code.ensure_loaded?

  * Read manifest when RunnerStats starts
  • Loading branch information
José Valim committed Feb 8, 2018
1 parent 6548704 commit 157913c
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 32 deletions.
21 changes: 8 additions & 13 deletions lib/ex_unit/lib/ex_unit/manifest.ex
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ defmodule ExUnit.Manifest do
File.write!(file, binary)
end

@spec load_from(Path.t()) :: t
def load_from(file) when is_binary(file) do
@spec read(Path.t()) :: t
def read(file) when is_binary(file) do
with {:ok, binary} <- File.read(file),
{:ok, {@manifest_vsn, manifest}} <- safe_binary_to_term(binary) do
manifest
Expand Down Expand Up @@ -76,27 +76,22 @@ defmodule ExUnit.Manifest do

defp prune_deleted_tests([], _, acc), do: acc

defp prune_deleted_tests([{{mod, name}, entry} | rest], file_existence, acc) do
file_exists = Map.fetch(file_existence, entry(entry, :file))
defp prune_deleted_tests([{{mod, name}, entry} | rest] = all, file_existence, acc) do
file = entry(entry, :file)
file_exists = Map.fetch(file_existence, file)

cond do
file_exists == :error ->
# This is the first time we've looked up the existence of the file.
# Cache the result and try again.
file_existence =
Map.put(file_existence, entry(entry, :file), File.regular?(entry(entry, :file)))

prune_deleted_tests([{{mod, name}, entry} | rest], file_existence, acc)
file_existence = Map.put(file_existence, file, File.regular?(file))
prune_deleted_tests(all, file_existence, acc)

file_exists == {:ok, false} ->
# The file does not exist, so we should prune the test.
prune_deleted_tests(rest, file_existence, acc)

function_exported?(mod, name, 1) ->
# The test still exists, at the same file, so keep it.
prune_deleted_tests(rest, file_existence, [{{mod, name}, entry} | acc])

Code.ensure_loaded?(mod) ->
Code.ensure_loaded?(mod) and not function_exported?(mod, name, 1) ->
# The test module has been loaded, but the test no longer exists, so prune it.
prune_deleted_tests(rest, file_existence, acc)

Expand Down
17 changes: 8 additions & 9 deletions lib/ex_unit/lib/ex_unit/runner_stats.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ defmodule ExUnit.RunnerStats do
@manifest ".ex_unit_results.elixir"

def init(opts) do
manifest_file =
{manifest_file, old_manifest} =
case Keyword.fetch(opts, :manifest_path) do
:error -> nil
{:ok, manifest_path} -> Path.join(manifest_path, @manifest)
:error ->
{nil, %{}}

{:ok, manifest_path} ->
manifest_file = Path.join(manifest_path, @manifest)
{manifest_file, Manifest.read(manifest_file)}
end

state = %{
Expand All @@ -19,7 +23,7 @@ defmodule ExUnit.RunnerStats do
skipped: 0,
excluded: 0,
manifest_file: manifest_file,
old_manifest: %{},
old_manifest: old_manifest,
new_manifest: %{}
}

Expand All @@ -35,11 +39,6 @@ defmodule ExUnit.RunnerStats do
{:reply, stats, state}
end

def handle_cast({:suite_started, _opts}, %{manifest_file: file} = state) when is_binary(file) do
state = %{state | old_manifest: Manifest.load_from(file)}
{:noreply, state}
end

def handle_cast({:test_finished, %Test{} = test}, state) do
state =
state
Expand Down
12 changes: 6 additions & 6 deletions lib/ex_unit/test/ex_unit/manifest_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,20 @@ defmodule ExUnit.ManifestTest do

@manifest_path "example.manifest"

describe "write!/2 and load_from/1" do
describe "write!/2 and read/1" do
test "can roundtrip a manifest", context do
manifest = non_blank_manifest()

in_tmp context.test, fn ->
assert write!(manifest, @manifest_path) == :ok
assert load_from(@manifest_path) == manifest
assert read(@manifest_path) == manifest
end
end

test "returns a blank manifest when loading a file that does not exit" do
path = tmp_path() <> "missing.manifest"
refute File.exists?(path)
assert load_from(path) == %{}
assert read(path) == %{}
end

test "returns a blank manifest when the file cannot be read", context do
Expand All @@ -99,7 +99,7 @@ defmodule ExUnit.ManifestTest do
stat = %{stat | mode: 0}
File.write_stat!(@manifest_path, stat)

assert load_from(@manifest_path) == %{}
assert read(@manifest_path) == %{}
end
end

Expand All @@ -111,7 +111,7 @@ defmodule ExUnit.ManifestTest do
corrupted = "corrupted" <> File.read!(@manifest_path)
File.write!(@manifest_path, corrupted)

assert load_from(@manifest_path) == %{}
assert read(@manifest_path) == %{}
end
end

Expand All @@ -123,7 +123,7 @@ defmodule ExUnit.ManifestTest do
assert {vsn, ^manifest} = @manifest_path |> File.read!() |> :erlang.binary_to_term()
File.write!(@manifest_path, :erlang.term_to_binary({vsn + 1, manifest}))

assert load_from(@manifest_path) == %{}
assert read(@manifest_path) == %{}
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/ex_unit/test/ex_unit/runner_stats_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ defmodule ExUnit.RunnerStatsTest do
simulate_test(formatter, :test_2, :failed)
end)

assert load_manifest() == %{
assert read_manifest() == %{
{TestModule, :test_1} => entry(file: __ENV__.file, last_run_status: :passed),
{TestModule, :test_2} => entry(file: __ENV__.file, last_run_status: :failed)
}
Expand All @@ -63,7 +63,7 @@ defmodule ExUnit.RunnerStatsTest do
simulate_suite(&simulate_test(&1, :test_1, :passed))
simulate_suite(&simulate_test(&1, :test_2, :failed))

assert load_manifest() == %{
assert read_manifest() == %{
{TestModule, :test_1} => entry(file: __ENV__.file, last_run_status: :passed),
{TestModule, :test_2} => entry(file: __ENV__.file, last_run_status: :failed)
}
Expand Down Expand Up @@ -100,7 +100,7 @@ defmodule ExUnit.RunnerStatsTest do
defp state_for(:skipped), do: {:skipped, "reason"}
defp state_for(:excluded), do: {:excluded, "reason"}

defp load_manifest do
Manifest.load_from(@manifest_file)
defp read_manifest do
Manifest.read(@manifest_file)
end
end

0 comments on commit 157913c

Please sign in to comment.