toHaveElementsAndAll + feature produces non-grouped output

[vlsi]

Affected Version

0.18.0

API

fluent-en_GB

Platform

jvm

Kotlin Version

1.8

How to reproduce the problem?

expect(listOf(Throwable("hello"))) {
    toHaveElementsAndAll {
        message.toContain("world")
    }
}

Describe the bug

There seem to be several issues:

1. message: branch is duplicated in the output.
2. I don't understand why "to be an instance of type" appears
3. It looks like the value for "message" is not displayed

I expected subject: [java.lang.Throwable: hello]        (java.util.Collections.SingletonList <1506951181>)
◆ elements need all: 
    » ▶ message: 
        ◾ to be an instance of type: String (kotlin.String) -- Class: java.lang.String
    » ▶ message: 
        ◾ to contain: 
          ⚬ value: "world"        <207471778>
              » but no match was found
    ❗❗ following elements were mismatched: 
       ⚬ index 0: kotlin.Throwable

Expected behaviour

I think to be an instance of type should not be displayed, and, in case it is displayed, it should be rendered under a common message: group like in

I expected subject: [java.lang.Throwable: hello]        (java.util.Collections.SingletonList <1506951181>)
◆ elements need all: 
    » ▶ message: hello <-- value should be here
        ◾ to be an instance of type: String (kotlin.String) -- Class: java.lang.String
        ◾ to contain: 
          ⚬ value: "world"        <207471778>
              » but no match was found
    ❗❗ following elements were mismatched: 
       ⚬ index 0: kotlin.Throwable

---

This looks much better:

expect(AbstractMap.SimpleEntry("a", Throwable("hello"))) {
    value.message.toContain("world")
}

=>

I expected subject: a=java.lang.Throwable: hello        (java.util.AbstractMap.SimpleEntry <1301987508>)
◆ ▶ value: kotlin.Throwable
    ◾ ▶ message: "hello"        <408069119>
        ◾ to contain: 
          ⚬ value: "world"        <1912850431>
              » but no match was found

[vlsi]

if I use message {...}, the output is better: message: is properly grouped in the report

expect(listOf(Throwable("hello"))) {
    toHaveElementsAndAll {
        message {
            toContain("world")
        }
    }
}

=>

I expected subject: [java.lang.Throwable: hello]        (java.util.Collections.SingletonList <1131592118>)
◆ elements need all: 
    » ▶ message: 
        ◾ to be an instance of type: String (kotlin.String) -- Class: java.lang.String
        ◾ to contain: 
          ⚬ value: "world"        <908084672>
              » but no match was found
    ❗❗ following elements were mismatched: 
       ⚬ index 0: kotlin.Throwable

[vlsi]

to be an instance of type: comes from notToEqualNull():

val <T : Throwable> Expect<T>.message: Expect<String>
    get() = feature(Throwable::message).notToEqualNull()
...
inline fun <reified T : Any> Expect<T?>.notToEqualNull(): Expect<T> =
    notToEqualNullButToBeAnInstanceOf(T::class).transform()

[vlsi]

The feature name is printed several times due to

atrium/atrium-core/src/commonMain/kotlin/ch/tutteli/atrium/creating/impl/FeatureExpectImpl.kt

Lines 61 to 80 in 65227ad

	//Would be nice if we don't have to add it immediately to the previousExpect but only:
	//if (!assertion.holds()) {
	//this way we could show chained features as one in reporting, i.e. message.contains("a").contains("b") would be:
	//* > message
	// * contains: "a"
	// " contains: "b"
	//and not
	//* > message
	// * contains: "a"
	//* > message
	// " contains: "b"
	//
	//However, for this to work we would need to know when no more assertion is defined. This would be possible
	//for CollectingExpectImpl
	(previousExpect as AssertionContainer<*>).append(
	assertionBuilder.feature
	.withDescriptionAndRepresentation(description, representation)
	.withAssertions(ArrayList(assertions))
	.build()
	)

[robstoll]

Thanks for your report. I see most things you have already figured out yourself.
For the notToEqualNull topic, please take a look at #330 and state your wishes there.

For:

» ▶ message: hello <-- value should be here

the » bullet point signifies an explanation of an expectation. hello should not be shown there as there could be multiple mismatches. But it should show up in the mismatch part. I.e. here:

⚬ index 0: kotlin.Throwable
    ▶ message: "hello"

I thought I already created an issue but apparently I only had it in my head.
Created a new issue: #1360
please state your wishes/thoughts there.

Regarding the grouping. We could already improve the implementation whenever the fail fast syntax is used within an expectation group. But we shouldn't do it in all cases IMO. I think it would make sense if the feature appears in sequence .
Like in your example, or if one writes:

expect { throwableFunction }.toThrow<IllegalArgumentException> {
  message.toContain("foo").toContain("bar") // will produce two feature assertions which can be merged into one
}

However, I wouldn't do it in the following case

expect { throwableFunction }.toThrow<MyException> {
   message.startsWith("expected prefix")
   cause.toEqual(null)
   message.toContain("foo bar")
}

I guess there is a reason why one chooses to split it this way. i.e. in reporting I would show:

> message 
  * to start with: "expected prefix"
> cause: 
  * not to equal: null
> message 
  * to contain: "foo bar" 

---

I guess we would need a compiler plugin for the fail-fast use case:

expect { throwableFunction }.toThrow<IllegalArgumentException>()
  .message.toContain("foo").toContain("bar")

which would be turned into something like

expect { throwableFunction }.toThrow<IllegalArgumentException> { 
  failFast=true // would require https://github.com/robstoll/atrium/issues/805
  message.toContain("foo").toContain("bar")
}

The benefit of it would be that you always see the subsequent expectations you defined in reporting, regardless if you use fail-fast or non-fail-fast semantic

[robstoll]

closing this as it entails multiple different aspects which are all covered in other issues:

There seem to be several issues:

1. message: branch is duplicated in the output.
2. I don't understand why "to be an instance of type" appears
3. It looks like the value for "message" is not displayed

1. A syntax that can allow grouping of assertions without compounding #805
2. was improved with change report for notToEqualNull #330
3. "following elements were mismatched" hides the reason for the failure #1359