Soft asserts: check all then throw all failures

[Fuud]

Note: this pull-request based on #49

Idea

Lets take an example:

public class IntegrationTest extends Specification {
    def "test1"() {
        when:
            def result = postRequest();
            def body = result["body"] // here can be complicated extraction logic
            def code = result["code"]
        then:
            body == "hello!"
            code == 256
    }

    Object postRequest() {
        [body: "hi", code: 42]
    }
}

It fails with:

Condition not satisfied:

body == "hello!"
|    |
hi   false
     5 differences (16% similarity)
     h(i----)
     h(ello!)
 <Click to see difference>

    at IntegrationTest.test1(Testss.groovy:13)

It's enougth for small and fast unit tests. But for integration tests we should provide all information that we have. So it will be better to check all conditions and report all failures. This behaviour should be enabled only if user explicitly enable it (because there can be blocking operations based on previous conditions). For example:

@AssertionMode(AssertionType.CHECK_ALL_THEN_FAIL)
public class IntegrationTest extends Specification {
    def "test1"() {
        when:
            def result = postRequest();
            def body = result["body"] // here can be complicated extraction logic
            def code = result["code"]
        then:
            body == "hello!"
            code == 256
    }

    Object postRequest() {
        [body: "hi", code: 42]
    }
}

will be reported as

Condition not satisfied:

body == "hello!"
|    |
hi   false
     5 differences (16% similarity)
     h(i----)
     h(ello!)
 <Click to see difference>

    at IntegrationTest.test1(Testss.groovy:13)


Condition not satisfied:

code == 256
|    |
42   false
 <Click to see difference>

    at IntegrationTest.test1(Testss.groovy:14)

Additional rules

• User can separate blocks by then-then-then chain, for example this will report only one failure:

@AssertionMode(AssertionType.CHECK_ALL_THEN_FAIL)
public class IntegrationTest extends Specification {
    def "test1"() {
        when:
            def result = postRequest();
            def body = result["body"] // here can be complicated extraction logic
            def code = result["code"]
        then:
            body == "hello!"
        then:
            code == 256
    }

    Object postRequest() {
        [body: "hi", code: 42]
    }
}

• AssertionMode annotation can be specified on method level to override class-level annotation (so this feature can be enabled or disabled for specific method).

Other implementations

• JUnit has org.junit.rules.ErrorCollector rule. http://junit.org/javadoc/4.9/org/junit/rules/ErrorCollector.html
• TestNG has SoftAsserts. http://beust.com/weblog/2009/06/25/soft-asserts/

[jirutka]

+1

[szpak]

For a Spock user wanting to use soft assertion for the first time it would very useful to have a JavaDoc with sample usage here (with @link from AssertionMode and AssertionType).

[szpak]

I like the simplicity of your implementation.

[Fuud]

Thank you @szpak .
I updated pull request according your recomendations.

[Fuud]

@pniederw Any progress?

[leonard84]

+1
@Fuud how will it deal with and: blocks? will they behave like a single then: or like two?

then:
foo == true
and:
bar == false

[Fuud]

@leonard84
In this implementation and: does not separates blocks. May be it should.

FYI:
You can try this fork:

   <repositories>
         ....
        <repository>
            <id>fuud.forks</id>
            <name>Fedor Bobin's forks</name>
            <url>https://raw.githubusercontent.com/Fuud/mvn-repo/master/releases</url>
            <releases>
                <enabled>true</enabled>
                <updatePolicy>never</updatePolicy>
            </releases>
            <snapshots>
                <enabled>false</enabled>
            </snapshots>
        </repository>
    </repositories>

...

        <dependency>
            <groupId>org.spockframework</groupId>
            <artifactId>spock-core-fuud-fork</artifactId>
            <version>1.0-groovy-2.3</version>
            <scope>test</scope>
        </dependency>

[jerzykrlk]

This looks like a very useful feature - what is left to do here, to have it merged and released?

[leonard84]

@Fuud when thinking about it a bit more, I think it is preferable to have a single annotation without an enum. Since the enum has only two values and one of them is the default behavior of the validation anyway, I don't see an advantage by using it. I think it would be more clear if the annotation would be something along the lines of @CheckAllAssertions.

[leonard84]

Please fix the indenting (starting and closing """ should be on the same level) and use given or setup in the feature bodies.

[Fuud]

It does not matter since repo owners does not want to merge it.

[jirutka]

I agree with @leonard84 about single annotation like @CheckAllAssertions. However, what if I enable it on a class level and need to disable it only on one test method?

[leonard84]

@jirutka I would say it is like @Ignore if you put it on classlevel then it applies to all methods there is no @NotIgnore either. If you really think we need it, then I would say @CheckAllAssertion(false) would be a good way to disable it again.

[jirutka]

@leonard84 I’m not sure if we need it, just pointing to a potential limitation. Something like @CheckAllAssertion(false) would be fine.

[Fuud]

Some problems with nested scopes. For example this test fails because condition inside Closure is recored by ErrorCollector.

@AssertionMode(AssertionType.CHECK_ALL_THEN_FAIL)
    def "test"(){
        setup:
            PollingConditions pollingConditions = new PollingConditions()
            AtomicInteger x = new AtomicInteger()

        expect:
            pollingConditions.eventually {
                x.incrementAndGet()
                x.get() == 2
            }
    }

Will be fixed soon.

[robfletcher]

I really want to merge this but we need to settle…

1. The specifics of the annotation — I'm all for a simple no-arg annotation that can be applied at method or class level.
2. The semantics of continuing blocks. I vote that assertions in each then block be grouped and evaluated together. Thus if you have multiple then blocks or then followed by and you have multiple groups of assertions that will fail independently.
3. Edge cases like PollingConditions example above that muddy the waters. It doesn't make sense to me that the error collector should process nested conditions like that much less stop them working "normally".

I know there's been a long delay in getting anywhere with this issue but if we have solutions to those problems I'd really like to move forward.

[robfletcher]

I'm going to close this as we merged #557 which provides an alternate approach to the same thing.

[rtretyak]

It worth noting that the merged approach only allows to verifyAll conditions in a single then/and block. It does not replace annotation approach, they can perfectly compromise each other.
I'm still lacking this functionality, because there are plenty use cases, when you want to do sth like

...
then: "verify sth"
predicate1()
predicate2()
and: "verify even more"
predicate3()

and do not fail after the first 'then' block. This could not be achieved via verifyAll but can be achieved via annotation.