spockframework / spock

The Enterprise-ready testing and specification framework.
https://spockframework.org
Apache License 2.0
3.54k stars 466 forks source link

Spock Soft Assertions ( verifyall() ) does not work with Power Asserts, AssertThat from AssertJ, etc #855

Open StephenOTT opened 6 years ago

StephenOTT commented 6 years ago
veryifyAll {

    assert a == b
    assert x == y

}

The second assert will not be reached. It is the same issues if you use assertJ assertThat, or use assert that() from hamcrest. If you just use that() without the "assert" in front it works as expected

Here is a snippet of a example i am working with:

  @Unroll
  def "Message Events Check: External Implementation"(){
    expect:
      verifyAll {
        that(event['implementation']['topic'], notNullValue())
        that(event['eventId'], notNullValue())
        that(event['eventName'], notNullValue())
      }
   where:
   event << model.getMessageEvents().findAll { it['implementation']['camundaType'] == 'external'}
}

If you add "assert" in front of the that() assert that() only the first assertion will function

leonard84 commented 6 years ago

verifyAll already applies implicit power assertions so explicit assert statements are unnecessary and seem to cause problems. AssertJ has its own soft assertions so combining them with verifyAll is not really usefull.

Btw:

    verifyAll {
        event['implementation']['topic'] != null
        event['eventId']  != null
        event['eventName'] != null
    }

is the spock way.

StephenOTT commented 6 years ago

@leonard84 The reason for the mixing of the frameworks was Spock provided better error introspection for deeply nested and lines such as:

...
where:
   event << model.getMessageEvents().findAll { it['implementation']['camundaType'] == 'external'}

and hamcrest/assertJ provided much easier to use validators than writing out the spock way, as you shown above.


verifyAll already applies implicit power assertions

@leonard84 Is this documented? Link? and is this the pattern of spock, that power assertions are always implicit? again this is documented?


My workaround for this will be to just use assertJ softly assertions such as

    SoftAssertions.assertSoftly { softly ->
      softly.assertThat(dog).withFailMessage("Dog cannot be null").isNotNull()
      softly.assertThat(cat).withFailMessage("cat cannot be null").isNotNull()
    }

and fix the where statement to provide objects that assertj can introspect ⚡️

Thanks!

szpak commented 6 years ago

I see it useful in all those cases where assert is required in Spock. Mostly assertions extracted to a separate methods (for reusability) or assertions done inside a closure. There verifyAll cannot be used - only the first assertion is used (with the assert keyboard) or no assertion is perform (with no assert keyboard).

(as the answer to the original report)

StephenOTT commented 6 years ago

Mostly assertions extracted to a separate methods (for reusability) or assertions done inside a closure. There verifyAll cannot be used - only the first assertion is used (with the assert keyboard) or no assertion is perform (with no assert keyboard).

+100 on that point. The reusability is key here, where i also have another use case, where i have assertions inside of Traits that i would like to call such as:

verifyAll {
   someMethodInATrait()
}

where that trait is implemented in the class, and the assertions are inside of the Trait's method

StephenOTT commented 6 years ago

@leonard84 @szpak thinking about this more, I see the bigger question of: How can soft assertions be reused through something like traits.

leonard84 commented 6 years ago

verifyAll could use some docs update, currently it is buried in the release notes.

All of spocks AST-Transformations rely the Specification, changing that would be an extensive refactoring.

StephenOTT commented 6 years ago

@leonard84 any alternatives come to mind?

leonard84 commented 5 years ago

Actually Hamcrest is supported http://spockframework.org/spock/docs/1.2-RC3/all_in_one.html#_code_hamcrestsupport_expect_code

It may be possible to add support for Assertj in a similar manner, but that is not really on my agenda. I may be open to accept a PR (please ask first with regards on how to implement this).

szpak commented 5 years ago

I ran into that problem again today with the integration tests in my Gradle plugin:

expect:
    verifyAll {
        PIT_PARAMETERS_NAMES_NOT_SET_BY_DEFAULT.each {
            assert !result.standardOutput.contains("${it}=")
        }
    }

It's hard to leave out assert in a loop or each {}. As it is quite heavy functional test I would prefer no to rerun it for every (of 20) parameters.