spock icon indicating copy to clipboard operation
spock copied to clipboard

PollingConditions.eventually should work also if defined with def

Open macieg opened this issue 6 years ago • 8 comments

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.

macieg avatar Nov 22 '19 18:11 macieg

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?

szpak avatar Nov 23 '19 11:11 szpak

Sure, I'll prepare a PR

macieg avatar Nov 23 '19 14:11 macieg

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

macieg avatar Nov 23 '19 15:11 macieg

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. :-(

Vampire avatar Nov 26 '19 12:11 Vampire

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

szpak avatar Nov 26 '19 19:11 szpak

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

szpak avatar Jan 09 '20 22:01 szpak

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.

leonard84 avatar Jan 10 '20 23:01 leonard84

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

thokari avatar Jun 02 '20 08:06 thokari