spockframework / spock

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

PollingConditions.eventually should work also if defined with def #1054

Open macieg opened 4 years ago

macieg commented 4 years ago

Hi, in the below's example I've provided two tests: first is going to pass, second is going to fail

The only difference is the way of creating PollingConditions object

class Playground extends Specification {

    def "not boom"() {
        given:
        def conditions = new PollingConditions()

        when:
        def x = 10

        then:
        conditions.eventually {
            1 == 2
        }
    }

    def "boom 2"() {
        given:
        PollingConditions conditions = new PollingConditions()

        when:
        def x = 10

        then:
        conditions.eventually {
            1 == 2
        }
    }
}

Is it expected behavior? There is nothing about it in the documentation of @ConditionBlock, which I strongly believe is related to that (we don't need to write assert keyword in the clojure).

If it's related to groovy itself it might be a good idea to fail when such usage is detected.

szpak commented 4 years ago

I've been debugging it years ago and after the code analysis it was problematic for Spock to determine that def is in fact the PollingConditions type. Maybe it will be easier to implement in Spock 2. As a workaround you need to explicitly add assert in the eventually block. I've mentioned it in my old presentation, but I don't see any related issue, so that could stay for a reference.

Could you make a PR to mention that in the documentation part for PollingConditions?

macieg commented 4 years ago

Sure, I'll prepare a PR

macieg commented 4 years ago

https://github.com/spockframework/spock/pull/1055

Vampire commented 4 years ago

Besides that the PR has "clojure" instead of "closure", the whole PollingConditions is not documented in the user guide, only mentioned in the change log. :-(

szpak commented 4 years ago

@Vampire Could you create a separate issue to document PollingConditions (or even better create a PR with an update :) )?

szpak commented 4 years ago

Documentation is updated. I changed the title to reflect that it could be potentially improved to support definition by def one day.

leonard84 commented 4 years ago

IIRC the issue is that we need to know the type of the receiver of the closure, so that we can perform compile time AST transformations on it. def is just an alias for Object and figuring out at runtime what type it is. So with def we don't actually know that the method eventually belongs to PollingConditions and is annotated with @ConditionBlock.

thokari commented 4 years ago

For now, I mentioned this caveat in the pending documentation for PollingConditions (https://github.com/spockframework/spock/pull/1173/files#diff-165ed30250fa253fe29f4c4e7c1467e7R77).