report all expectations within an expectation-group block

[robstoll]

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

Code related feature

I want to provide a between function which reports lower and upper bound as soon as one of them fails. Yet, I don't want to activate report all expectations by default (i.e. change reporting). I kind of want that the between expectation is considered as one block.

In Atrium terms, I want an easy way to define a summary group

expect(today).between(tomorrow, endOfMonth)

Would then show in reporting:

I expected subject: today
❌ to be after: tomorrow
✔️ to be before: endOfMonth

TODO:

• add report: ReportOptions.() -> Unit as parameter in every place where we currently have assertionCreators:
  for instance, to and (and group/expectGrouped, see Add soft assertions for verifying unrelated subjects #1330) s so that one can define between as:

  fun Expect<T>.between(t1: T, t2: T) = and(report = { showSummary() }) {
    toBeLessThan(t1)
    toBeGreaterThan(2)
  }
  

• consider to use the functionality for ch.tutteli.atrium.logic.impl.DefaultMapEntryAssertions#isKeyValue

[vlsi]

What do you think of creating a property/function on Expect instead of adding parameters all over the place?

[vlsi]

I guess this issue duplicates #724

[robstoll]

#724 is related but not the same.

there are already functions to modify the behaviour on an Expect level (all experimental):
https://github.com/robstoll/atrium/blob/main/apis/fluent-en_GB/atrium-api-fluent-en_GB/src/commonMain/kotlin/ch/tutteli/atrium/api/fluent/en_GB/expectExtensions.kt

but we don't provide it at arbitrary places.

I think we still need an option on the expectation function level because otherwise you will incorporate a side effect in your expectation function which might change the behaviour of other expectation functions by accident.

what I could imagine is to provide a way to modify report options within an expectation group. then one would kind of redefine the default options within that group and would still have the possibility to change it per function.
I need to think about this a as bit more.

[vlsi]

I think we still need an option on the expectation function level because otherwise you will incorporate a side effect in your expectation function which might change the behaviour of other expectation functions by accident.

I do not follow you here. There's an option to configure the way the group is printed: all assertions, failing only. What is the difference between the option at expect level vs an option at "all" parameter? There's no difference, except a new parameter in "all" method would be harder to use and harder to discover

[robstoll]

Maybe some more context:

• you can currently exchange the Reporter where the default Reporter only reports failing expectations
• AssertionFormatter decide how an Assertion is formatted in the output
• each expectation function defines on its own what type of Assertion it uses. If you use an AssertionGroup to report multiple Assertions then you can define the AssertionGroupType which has an influence on how it is shown in reporting. Currently the ExplanatoryAssertionGroupType and the SummaryAssertionGroupType implement DoNotFilterAssertionGroupType, an interface which gives the hint to the AssertionFormatter to show it's assertions in any case.

Now imagine you would define the following:

fun Expect<T>.between(t1: T, t2: T) = 
  report = { showSummary() } // side effect, changes the behaviour for all subsequent expectation functions
  .and {
    toBeLessThan(t1)
    toBeGreaterThan(2)
  }

I would imagine that the showSummary does not change the Reporter but would change the default AssertionGroupType used when creating an AssertionGroup . Expectation functions which want to provide the possibility to configure whether a List- or SummaryAssertionGroupType is used would then query this config value.

With the above this would mean, that suddenly every expectation function which follows a call to between uses a Summary-instead of a ListAssertionGroupType in reporting. I see a high potential of wrong usages of this feature

For the same reason, I am hesitant to add an option for an expectation group, something like:

fun Expect<T>.between(t1: T, t2: T) =
  .and {
    report = { showSummary() } // side effect, changes the behaviour for all subsequent expectation functions within this expectation group
    toBeLessThan(t1)
    toBeGreaterThan(2)
  }

I am also not yet convinced this is really something one wants to define individually per expectation group. I think it is enough, if one can define this at the RootExpect where also options exist to exchange components (e.g. the Reporter).

I think we are safe to add it already on an expectation function level where it influences only this very specific expectation. Taking the example in the description. the report as parameter would only influence the AssertionGroupType used for the AssertionGroup created by the and expectation function.

[vlsi]

I think it is enough, if one can define this at the RootExpect

Feature-level things (e.g. nested expect that changes subject) are not much different from RootExpect.

[vlsi]

For the same reason, I am hesitant to add an option for an expectation group, something like:

fun Expect<T>.between(t1: T, t2: T) =
  .and {
    report = { showSummary() } // side effect, changes the behaviour for all subsequent expectation functions within this expectation group
    toBeLessThan(t1)
    toBeGreaterThan(2)
  }

What if the actions on assertionCreator: Expect<T>.() -> Unit were scoped to assertionCreator?
In other words, assertionCreator could be executed on a throw-away Expect, then its impact is not "inherited" after calls like .between(...). No side-effects leak outside of and {..} block then.

[robstoll]

Feature-level things (e.g. nested expect that changes subject) are not much different from RootExpect.

I agree in terms of creating expectations but not in terms of reporting. Do you think you will ever want to define:

expect(x)
  .address {
     report { showSummary() } // show summary by default in this group
  }
  .age {
     // don't show summary by default in this group
   }

instead of:

expect(x)
  .report { showSummary() } // show summary by default
  .address { ... }
  .age { ... }

No side-effects leak outside of and {..} block then.

That's exactly how I would implement it "// side effect, changes the behaviour for all subsequent expectation functions within this expectation group"
The risk of misuse is way lower than the global setting but it still has an effect on the expectation functions you use within the group. I actually guess that's OK as the creator of this function will see the effect immediately when he is using it.
However, I am not convinced yet, that we really need this feature.

[vlsi]

I would prefer a structured way.
In other words, I think I would always prefer groups when more than one thing is verified.

For instance:

expect(x) {
  showSummary() // for both address and age
  address { ... }
  age { ... }
}

expect(x) {
  address {
    showSummary() // just here
    ...
  }
  age { ... }
}

Frankly speaking, I would refrain from syntax like expect(x).report {}... because, well, it does not autoformat properly, and it is hard to see the structure.

expect(x)
  .report { showSummary() } // show summary by default
  .address { ... }
  .age { ... }

It is not significantly different from

expect(x)
  .address { ... }
  .or
  .age { ... }
  .and
  .age { ... }
  .and
  .not
  .age { ... }
  .please
  .no

[robstoll]

good point, I basically do the same, switch to group syntax as soon as I state more than one expectation.

We already have (the experimental) API:

expect(foo)
  .withOptions { ... }

which I don't want to move into the group as changing e.g. the Reporter in the middle will only cause trouble. I.e. this should only be doable on the RootExpect before the first expectation was created. So my example was wrong, it would more likely look as follows:

expect(foo)
  .withOptions { 
    report { showSummary() }
  }
  .and {
    ...
  }

We could change it to:

expect(foo, options = { report { showSummary() } }) {
  address { .... }
  age { .... }
}

but that would mean that e.g. changing the representation of the subject is a tiny bit more verbose than now:

expect(foo, options = { withRepresentation("my lovely numbers") }) {
  address { .... }
  age { .... }
}

vs. current

expect(foo).withRepresentation("my lovely numbers")
  .and {
    ...
  }

but we would have only one approach which IMO is better. WDYT?