search

bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
388 stars 178 forks source link

asserts.equals could give better diffs when comparing objects of same built-in type #333

Open sfreilich opened 2 years ago

sfreilich commented 2 years ago

The current implementation of asserts.equals(expected, actual) in unittest.bzl puts just str(value) for each of the values in the assertion failure message. It would be nice if it gave a more detailed analysis of where the values differed when the values being compared were both lists, dicts, or (multi-line) strings. (This is analagous to what Python does in unittest.TestCase.assertEqual.)

For list, it should provide a diff of the items which makes it clear which subsequences are shared, which appear just in expected, and which appear just in actual.

For dict, it should make it clear which keys are just in expected, which are just in actual, which are in both with the same value, and which are in both with different values.

For str, it should show a multi-line readable diff, possibly in unified diff format.

That would make these tests much easier to debug when the values in the assertions are complicated.

rickeylev commented 2 years ago

Yeah, big +1 to better diff output. As a concrete example, I wrote some tests that needed to verify sets were equal. While asserts.set_equals() does a diff, it stuffs it all on one line, and having to visually parse out "missing", "unexpected", and the start/end brackets, parens, etc is a huge pain.

In comparison, in order to check that one list contained all the values of another list, the failure text is multi-line with distinct "heres what was missing" and "heres what we checked" sections, and it is incredibly easy to visually see what is missing (or that some path was typoed, etc).

I think a small missing piece is a pretty printing function, too. I don't think that's possible in starlark because it prohibits recursion? Pretty printing would also make up for the lack of an actual diff -- visually comparing two structures, one above the other, from a simple "actual: X\n: expected: Y" output is much easier when pretty printed.

tetromino commented 2 years ago

PRs welcome for better list/dic diff output.

A unified diff for str values would be a rather complicated beast in pure Starlark (which we'd want it to be for an analysis test), and I suspect the cost isn't worth it.

sfreilich commented 2 years ago

Well, full unified diff might be excessive, but you have splitlines() and could probably manage some sort of multi-line diff.