Exclude UsageHintGroup from proof explanation rendering

[WesleyJammer]

Updated DefaultProofExplanationTextPreRenderer to filter out UsageHintGroup children when rendering proof explanations. Added a test to verify that usage hints are not included in the output for proof explanations.

---

I confirm that I have read the Contributor Agreements v1.0, agree to be bound on them and confirm that my contribution is compliant.

[WesleyJammer]

@robstoll here is the updated pull request. I did as you said and the merch conflicts are gone. i hope to see your review soon!

[robstoll]

Looks good to me, let's see if CI is happy as well, and then I suggest you continue on this branch/PR with the rest.

[robstoll]

@WesleyJammer oh... before you continue with debugGroup, please add a test where usageHintGroup is not a direct child

[WesleyJammer]

@robstoll what do you mean? Add an extra test with a ProofExplanation and then a seperate UsageHintGroup outside of the ProofExplanation?

[robstoll]

works only if UsageHintGroup is a direct child. As an example of a usageHintGroup see the MyClass example (unfortunately we currently don't have this hint but you can make it up in the test)
robstoll/atrium-roadmap#91 (comment)

now Imagine you define something like:

class MyClass(val i: Int)
class ExampleTest {
    @Test
    fun foo() {
        val listOfListOfMyClass = emptyList<List<MyClass>>()
        expect(listOfListOfMyClass).toContainExactly(
            { toContainExactly { toEqual(MyClass(12)) } },
            { toHaveElements() }
        )
    }
}

Current reporting would output (if we had the hint):

I expected subject : []        (kotlin.collections.EmptyList <1244678176>)
🚩️ ▶ size        : 0
     🚫️ to equal : 2
🚩️ to contain only, in order : 
   🚩️ ▶ element 0 : ❗❗ hasNext() returned false
        » ▶ size       : 
            • to equal : 1
        » to contain only, in order : 
          • ▶ element 0  : 
              • to equal : ch.tutteli.atrium.api.fluent.en_GB.MyClass@2f6f2d91        (ch.tutteli.atrium.api.fluent.en_GB.MyClass <795815313>)
                💡 notice, MyClass has not overridden equals
                   better use toBeTheInstance or implement equals/hashCode
   🚩️ ▶ element 1 : ❗❗ hasNext() returned false
        » to have : a next element

Hm... this example let me question if we really should remove usageHintGroup in a proofExplanation, maybe it is not so bad after all if the user sees it already here?
Anyway, there are other types which we should remove, so doesn't matter too much if we take UsageHintGroups as example, we can still adapt this later on. The main point is that it is not enough to filter out only direct children. You could go through all children as you did with filterNot but I guess that's not the most efficient approach. Maybe better to somehow track the Context we are currently in (I guess this approach will also enable other features in the future)

[WesleyJammer]

@robstoll i made some changes in this pullrequest. i made a test with a UsageHintGroup not as a direct child and now it is also not visible anymore. i hope this is more what you are looking for. otherwise let me know!

[robstoll]

@WesleyJammer looks better indeed 🙂👍
You can continue with the other types. Maybe worth introducing an abstract class something like DontShowIfExplainingTextPreRenderer which implements transformIt and requires subclasses to implement transformIfNotExplaning?

ps: I'll check why the apiCheck fails.

[robstoll]

@WesleyJammer please rebase, fixed the api issues

[WesleyJammer]

@robstoll okey and then i suppose i need to make a new pull request? with the new rebase of proof-based-reporter + my addons?

[robstoll]

@WesleyJammer not necessarily, you can keep this PR, rebase your branch and then git push --force-with-lease. And afterwards continue with the other types, create a new commit and push again. We can also merge the PR after the rebase if you prefer and you continue with the new changes in a new PR, that's fine as well

[WesleyJammer]

@robstoll done!

[robstoll]

@WesleyJammer the build failures are most likely again not related to your changes. Sorry about that, going to take a look

[robstoll]

@WesleyJammer fixed the compilation errors but it looks like I have not yet fixed all failing tests on this branch. Some are already fixed on main, so simplest would be if I rebase on main but that would mean that you have again merge conflicts. Would you mind if I do this nevertheless and you cherry-pick your changes afterwards again?

[WesleyJammer]

@robstoll i will do that.

[robstoll]

@WesleyJammer good, I'll let you know once I have rebased. You can continue in the meantime 🙂

[robstoll]

Closing this since #2132 is the successor