rethink reporting of subject changes

[robstoll]

Situation

We currently distinguish between feature extraction and a reporter subject change. They are very similar though. The main thing is a difference in reporting. Feature extraction uses an AssertionGroup with a FeatureAssertionGroupType where a subject change uses a descriptive assertion.

expect(listOf(1).get(0).toBe(2)

Following the comparison in reporting:

feature extraction:

expect: List(1)
* > get(0): 1
   * to be: 2

subject change (assuming get is implemented as subject change)

expect: List(1)
* has element at index: 0
* to be: 2

But... the check of the subject change does not even show up as it actually holds. So the error reporting looks like the following:

expect: List(1)
* to be: 2

Which is confusing. => hence feature extraction makes more sense.

---

But why having a reported subject change then? It was mainly introduced because of notToBeNull where the following

expect(1 as Int?).notToBeNull { toBe(1) }

would look like the following with feature extraction:

expect: 1         (Kotlin.Int)
* > is instance of: Int
    * to be: 2

vs. subject change

expect: 1         (Kotlin.Int)
* to be: 2

In terms of analysing the error the is instance of: Int seems redundant, as we can already see that from the first line. Thus I think it made sense to remove that. This is also true for isA and toThrow. For instance:

expect{
    throw IllegalArgumentException("a")
}.toThrow<IllegalArgumentException>{ messageContains("b")}

which looks like the following in reporting:

expect the thrown exception: java.lang.IllegalArgumentException
◆ ▶ message: "a"        <1630342910>
    ◾ contains: 
      ⚬ value: "b"        <1897115967>
        ⚬ ▶ number of occurrences: 0
            ◾ is at least: 1

no need for an: * is instance of: java.lang.IllegalArgumentException in addition. However, if one had a reporter which reports all assertions (not only failing) then the * is instance of: java.lang.IllegalArgumentException shows up.

Problem

1. A user has to make the choice what fits better and this is not always clear

For instance, type transformations such as isA (or notToBeNull) are implemented via subject change. Hence expect(Result.success(0)).isSuccess().toBe(1) which includes kind of a type check as well, would logically be implemented with a subject changer as well. However, reporting would look like the following:

 expect: Success(0)        (kotlin.Result <1518864111>)
◆ to be: 1

which could be confusing because of the hidden is: Success. Or consider the EitherSpec:

expect(Left("hello")).isLeft { startsWith("go") }

looks like the following:

expect: Left(hello)        (ch.tutteli.atrium.domain.builders.creating.Left <22931893>)
◆ starts with: "g"        <712469796>"        <436193654>

which might be confusing as well without the is a: Left

For this reasons I have decided that isSuccess should be implemented with a feature extraction (see https://github.com/robstoll/atrium/issues/203)

To be discussed

Shall we keep the distinction between subject change and feature extraction?

[robstoll]

Currently we use as... in the API when we do an unreported transformation. As this is somewhat related to this issue I mentioned it here as well. Is it worth to make the distinction in the API as we need to explain it to the user. For instance, we have values but asEntries on Map. Not intuitive.

We could also provide a report option for feature extractors instead which would be available for every feature extraction. and set the default in a sense-full way per function. For instance, for Map<K,V>.entries I would set the default to not show the extraction. Of course this only moves the problem to another spot.