Add hints for well-known differences like "whitespace-only changes", "end-line-markers differ", "unified diff|"

[vlsi]

Platform (all, jvm, js): all
Extension (none, kotlin 1.3): none

Code related feature

expect(actualYamlString).toEqual("long-yaml-string")

// or

expect(actualXmlString).toEqual("long-xml-string")

Currently, Atrium prints "expected ... to equal ...", however, it is hard to tell what was not matching.
It would be nice if there were automatic hints that could summarize the nature of the difference.

For instance,

• If the only difference is end-of-line markers (CR vs CRLF), there could be a hint "expected string uses CRLF while actual string uses LF end-of-line markers"
• If the "unified diff" of the expected and actual is less than 50% of min(expected, actual), then it would probably worth displaying the unified diff. In other words, if the expected and actual contain ~50 lines (e.g. 50 lines of yaml), and only a few of them differ, then it would be nice to see what was actually the mismatch
• If the "unified diff" is big (e.g. some changes + indentation changes), it might be worth trying to run a ignore whitespace changes and run a unified diff. If that produces small diff, Atrium could highlight "here's a diff ignoring the whitespace changes"
• There might be a case for calling into native diff tools like https://github.com/Wilfred/difftastic by @Wilfred so the diff might be easier to understand in case it is caused by "added yaml node" or "added xml node"

PS. This request is related to "click to show the difference" in IDE, however, this request would help users in CI as well. For instance, you often can't "click to see the difference" in CI logs

See also

• hints for data classes if using toEqual #151
• warn if a CharSequence.contains and the like did not match because of different line endings #834

[robstoll]

I totally agree (see robstoll/atrium-roadmap#39 for further ideas regarding a diff in the IDE). I would like to split the issue into two, the CR vs CRLF hint could be tackled easily (there is already an issue: #834) and be done in addition to the diff is a bit harder

[vlsi]

Are there generic ways to add hints or should I try treating .toEqual explicitly?
I'm interested in fixing diff first as it impacts me more than crlf

[robstoll]

I am not sure what you mean by "a generic way to add a hint".
In the end we need to adjust DefaultAnyAssertions.toBe, something along the line of

    override fun <T> toBe(container: AssertionContainer<T>, expected: T): Assertion {
        val expect = container.toExpect()
        return assertionBuilder.descriptive
            .withTest(expect) { it == expected }
            .withHelpOnFailureBasedOnDefinedSubject(expect) { subject ->
                if (subject is CharSequence && expected is CharSequence) {
                    // TODO we should figure out first how it should look like in reporting
                    assertionBuilder.representationOnly.failing.withRepresentation(Untranslatable("...")).build()
                }
                //TODO allow that one can return null as well so that we can choose in here whether we actually want to
                // create the failure hint or not
            }
            .withDescriptionAndRepresentation(TO_EQUAL, expected)
            .build()
    }

of course, this will only cover the toEqual case but not contains (as shown in #834)

[robstoll]

@vlsi Just came to my mind that we already have a ShowOption which allows to define when the failure hint shall be shown and in which case not. In the above it was defined that it shall be shown if the subject is defined. (done within withHelpOnFailureBasedOnDefinedSubject)
Use the following instead

    override fun <T> toBe(container: AssertionContainer<T>, expected: T): Assertion {
        val expect = container.toExpect()
        return assertionBuilder.descriptive
            .withTest(expect) { it == expected }
            .withHelpOnFailureBasedOnSubject(expect) {
                ifDefined { subject ->
                    val charSequenceSubject = subject as CharSequence
                    // TODO we should figure out first how it should look like in reporting
                    assertionBuilder.representationOnly.failing.withRepresentation(Untranslatable("...")).build()
                }.ifAbsent(::createShouldNotBeShownToUserWarning)
            }.showBasedOnDefinedSubjectOnlyIf(expect) {
                it is CharSequence && expected is CharSequence
            }
            .withDescriptionAndRepresentation(TO_EQUAL, expected)
            .build()
    }

[robstoll]

@vlsi I have added a paramshowOnlyIfto withHelpOnFailureBasedOnDefinedSubject. You can use the following:

    override fun <T> toBe(container: AssertionContainer<T>, expected: T): Assertion {
        val expect = container.toExpect()
        return assertionBuilder.descriptive
            .withTest(expect) { it == expected }
            .withHelpOnFailureBasedOnDefinedSubject(
                expect,
                showOnlyIf = { subject -> subject is CharSequence && expected is CharSequence }
            ) { subject ->
                val charSequenceSubject = subject as CharSequence
                if ( whitespaceOnlyDiff ) {
                  // TODO we should figure out first how it should look like in reporting
                  assertionBuilder.representationOnly.failing.withRepresentation(Untranslatable("...")).build()
                }
            }
            .withDescriptionAndRepresentation(TO_EQUAL, expected)
            .build()
    }