Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 9 additions & 2 deletions lib/ex_unit/lib/ex_unit/diff.ex
Original file line number Diff line number Diff line change
Expand Up @@ -558,15 +558,15 @@ defmodule ExUnit.Diff do
end

defp move_right({y, list1, [elem2 | rest2], {edit1, edit2, env}}) do
{y, list1, rest2, {edit1, [{:ins, elem2} | edit2], env}}
{y, list1, rest2, {edit1, [{:ins, build_elem(elem2)} | edit2], env}}
end

defp move_right({y, list1, [], edits}) do
{y, list1, [], edits}
end

defp move_down({y, [elem1 | rest1], list2, {edit1, edit2, env}}) do
{y + 1, rest1, list2, {[{:del, elem1} | edit1], edit2, env}}
{y + 1, rest1, list2, {[{:del, build_elem(elem1)} | edit1], edit2, env}}
Copy link
Member

Choose a reason for hiding this comment

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

I am worried this change is happening too deep in the algorithm. We should probably deal with converting to struct/map when building the diff script?

Copy link
Contributor Author

@dkuku dkuku Nov 3, 2024

Choose a reason for hiding this comment

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

I can try to move it where it is extracted from the diff later, it still may be not the best place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'ts enough to convert it in the escape function, it is called in the diff_improper when the non matched elem is inserted on the right (line 401) and list_script_to_diff when it's in :del on left side on line 621

end

defp move_down({y, [], list2, edits}) do
Expand Down Expand Up @@ -763,6 +763,10 @@ defmodule ExUnit.Diff do
_ -> :error
end

def build_elem(elem) when is_struct(elem), do: elem
def build_elem(elem) when is_map(elem), do: {:%{}, [], Map.to_list(elem)}
def build_elem(elem), do: elem

defp maybe_struct(%name{}), do: name
defp maybe_struct(_), do: nil

Expand Down Expand Up @@ -1145,6 +1149,9 @@ defmodule ExUnit.Diff do
# We escape container types to make a distinction between AST
# and values that should be inspected. All other values have no
# special AST representation, so we can keep them as is.
# Maps should be formatted not inspected.
defp escape(other) when is_struct(other), do: {other}
defp escape({:%{}, _, _} = other), do: other
defp escape(other) when is_list(other) or is_tuple(other), do: {other}
defp escape(other), do: other

Expand Down
52 changes: 52 additions & 0 deletions lib/ex_unit/test/ex_unit/formatter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,58 @@ defmodule ExUnit.FormatterTest do
"""
end

test "formats nested maps with column limit" do
Copy link
Member

Choose a reason for hiding this comment

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

Could we have a test in diff_test.exs instead?

Copy link
Contributor Author

@dkuku dkuku Nov 3, 2024

Choose a reason for hiding this comment

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

Good shout, updated

map =
%{
microsecond: {1, 2},
second: 3,
month: 4,
day: 5,
year: 2024,
minute: 6,
hour: 7
}

failure = [{:error, catch_assertion(assert [map, %{map | hour: 8}] == [map]), []}]

assert format_test_all_failure(test_module(), failure, 1, 80, &formatter/2) == """
1) Hello: failure on setup_all callback, all tests have been invalidated
Assertion with == failed
code: assert [map, %{map | hour: 8}] == [map]
left: [
%{
microsecond: {1, 2},
second: 3,
month: 4,
day: 5,
year: 2024,
minute: 6,
hour: 7
},
%{
microsecond: {1, 2},
second: 3,
month: 4,
day: 5,
year: 2024,
minute: 6,
hour: 8
}
]
right: [
%{
microsecond: {1, 2},
second: 3,
month: 4,
day: 5,
year: 2024,
minute: 6,
hour: 7
}
]
"""
end

test "formats assertions with complex function call arguments" do
failure = [{:error, catch_assertion(assert is_list(List.to_tuple([1, 2, 3]))), []}]

Expand Down