Fail explanatory assertion group on empty assertionCreator

[wordhou]

Resolves #933. Whenever an assertionCreator is taken as an argument of an expectation, an explanatory group is created that describes the assertions created. If no assertions are created, then an empty lambda failure hint is given. This PR makes the explanatory group fail when the empty lambda failure hint is given, and then removes the createEmptyAssertionHintIfNecessary from the assertion creators.

---

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

[wordhou]

Would it be better to make collectAssertions(...) fail the explanatory group when the assertionCreator doesn't create any assertions?

[robstoll]

sounds like a reasonable thing. That an explanatory group can fail is rather a new thing

[wordhou]

I realize I'm not actually sure how to modify the builder to accomplish this.

I'm looking at AssertionOptionExplanatoryExtensions.kt and I'm not sure how to get the extension method .collectAssertions to produce a failing final step.

My naive attempt:

fun <T, G : ExplanatoryAssertionGroupType, R> AssertionsOption<G, R>.collectAssertions(
    container: AssertionContainer<*>,
    maybeSubject: Option<T>,
    assertionCreator: Expect<T>.() -> Unit
): R {
    val collectingExpect = CollectingExpect<T>(None, container.components)
    // not using addAssertionsCreatedBy on purpose so that we don't append a failing assertion
    collectingExpect.assertionCreator()
    return withAssertions(container.collectForCompositionBasedOnSubject(maybeSubject, assertionCreator))
        .let {
            if(collectingExpect.getAssertions().isEmpty()) it.failing
            else it
        }
}

has issues because the return type is the generic type R and it doesn't recognize .failing. I'm not really understanding here how the builder pattern is implemented here with the generic AssertionsOption<G,R> type.

[robstoll]

You need to restrict R with an upper bound of ExplanatoryGroup.FinalStep.kt in AssertionOptionExplanatoryExtensions.kt.
So like the following:

fun <T, G : ExplanatoryAssertionGroupType, R : ExplanatoryGroup.FinalStep> AssertionsOption<G, R>.collectAssertions(...

[wordhou]

With the above type signature the compiler complained about two things:

• It expects the method to return R but the inferred type is ExplanatoryGroup.FinalStep
• Elsewhere in the code, in 4 other places, it complains about the upper bound R : ExplanatoryGroup.FinalStep, saying ExplanatoryAssertionGroupFinalStep isn't a subtype of ExplanatoryGroup.FinalStep. I think the problem is that the type options withDefaultType, withWarningType, withInformationType and withType all have a type signature AssertionsOption<T, ExplanatoryAssertionGroupFinalStep>, at least for now. I saw your notes about replacing ExplanatoryAssertionGroupFinalStep with ExplanatoryGroup.FinalStep before version 1.0.0.

So I had to use the following to satisfy the compiler.

fun <T, G : ExplanatoryAssertionGroupType, R : ExplanatoryAssertionGroupFinalStep> AssertionsOption<G, R>.collectAssertions(
    container: AssertionContainer<*>,
    maybeSubject: Option<T>,
    assertionCreator: Expect<T>.() -> Unit
) : ExplanatoryAssertionGroupFinalStep {
  // ...
}

However this generates a warning on compilation. Am I approaching this in the right way? Do we suppress/ignore the warning or do something else?

[codecov]

Codecov Report

Merging #938 (568a1bf) into master (e73e465) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #938      +/-   ##
==========================================
+ Coverage   91.34%   91.40%   +0.06%     
==========================================
  Files         429      429              
  Lines        4299     4295       -4     
  Branches      219      218       -1     
==========================================
- Hits         3927     3926       -1     
+ Misses        323      320       -3     
  Partials       49       49              

Flag	Coverage Δ
bbc	80.79% <100.00%> (-0.02%)	⬇️
bc	80.70% <100.00%> (-0.01%)	⬇️
current	87.63% <100.00%> (+0.05%)	⬆️
current_windows	86.70% <100.00%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...creators/impl/InAnyOrderEntriesAssertionCreator.kt	97.29% <ø> (-0.53%)	⬇️
...collectors/AssertionsOptionExplantoryExtensions.kt	100.00% <100.00%> (ø)
...ium/api/infix/en_GB/chronoLocalDateExpectations.kt	100.00% <0.00%> (ø)
.../api/infix/en_GB/chronoZonedDateTimeExpectation.kt	100.00% <0.00%> (ø)
...api/infix/en_GB/chronoLocalDateTimeExpectations.kt	100.00% <0.00%> (ø)
...pi/fluent/en_GB/chronoZonedDateTimeExpectations.kt	100.00% <0.00%> (ø)
...logic/impl/DefaultChronoZonedDateTimeAssertions.kt	87.50% <0.00%> (+7.50%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e73e465...568a1bf. Read the comment docs.

[robstoll]

@wordhou sorry, I am quite busy at the moment, going to look into it hopefully soon