New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test failure with null when multiple nested with blocks #778

Closed
cylon opened this Issue Oct 4, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@cylon

cylon commented Oct 4, 2017

Issue description

When using deeply nested with() blocks the test fails with a null value

Condition not satisfied:

with(it[0]) { with(it[0]) { it == 'end' } }
|    | |
null | [end]
     [[end]]

How to reproduce

https://github.com/cylon/spockwithtest

Additional Environment information

gradle 4.2

Java/JDK

java version "1.8.0_144"
Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)

Groovy version

2.4.10

Build tool version

Gradle


Gradle 4.2

Build time: 2017-09-20 14:48:23 UTC
Revision: 5ba503cc17748671c83ce35d7da1cffd6e24dfbd

Groovy: 2.4.11
Ant: Apache Ant(TM) version 1.9.6 compiled on June 29 2015
JVM: 1.8.0_144 (Oracle Corporation 25.144-b01)
OS: Windows 10 10.0 amd64

Operating System

Windows

Build-tool dependencies used

Gradle/Grails

compile 'org.codehaus.groovy:groovy-all:2.4.10'
testCompile 'org.spockframework:spock-core:1.1-groovy-2.4'
@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Oct 4, 2017

Member

According to your example project this works with spock-1.0 and not anymore with spock-1.1 correct? I'm guessing that it was broken by 21002e6 (PR #606) however this needs more investigation. @angry-cellophane could you look at it?

Member

leonard84 commented Oct 4, 2017

According to your example project this works with spock-1.0 and not anymore with spock-1.1 correct? I'm guessing that it was broken by 21002e6 (PR #606) however this needs more investigation. @angry-cellophane could you look at it?

@angry-cellophane

This comment has been minimized.

Show comment
Hide comment
@angry-cellophane

angry-cellophane Oct 4, 2017

Contributor

@leonard84 looking into the issue

Contributor

angry-cellophane commented Oct 4, 2017

@leonard84 looking into the issue

@cylon

This comment has been minimized.

Show comment
Hide comment
@cylon

cylon Oct 5, 2017

Yes, that is correct in 1.0 it worked and in 1.1 it doesn't.

cylon commented Oct 5, 2017

Yes, that is correct in 1.0 it worked and in 1.1 it doesn't.

@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Oct 10, 2017

Member

@angry-cellophane the problem you described was this line SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])

Because the method is invoked on this but not the closure it leads to an error (Method not found).

Did you try to change it to
SpockRuntime.verifyMethodCondition(this, "contains", [1])
instead?

Member

leonard84 commented Oct 10, 2017

@angry-cellophane the problem you described was this line SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])

Because the method is invoked on this but not the closure it leads to an error (Method not found).

Did you try to change it to
SpockRuntime.verifyMethodCondition(this, "contains", [1])
instead?

@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Oct 16, 2017

Member

git bisect verified that 21002e6 is the culprit. When I reverted this commit it fixed the problem for master.

So we need a better way then to use the identity closure workaround.

Member

leonard84 commented Oct 16, 2017

git bisect verified that 21002e6 is the culprit. When I reverted this commit it fixed the problem for master.

So we need a better way then to use the identity closure workaround.

@angry-cellophane

This comment has been minimized.

Show comment
Hide comment
@angry-cellophane

angry-cellophane Oct 16, 2017

Contributor

Finally had enough time to look into the issue.

SpockRuntime.verifyMethodCondition(this, "contains", [1])

the code above doesn't do what we want: to invoke the contains method on the closure. If we leave this groovy will use a reference to the spec at this place.

The bug was caused by the PR #606
The problem with the with expressions is that they are part of spec, with is a method of spock.lang.Specification and should always be invoked on the spec. The PR changed that behavior.
I've raised another PR #782 to exclude the with expressions from being invoked on the closure and leave them being invoked on the spec.

Contributor

angry-cellophane commented Oct 16, 2017

Finally had enough time to look into the issue.

SpockRuntime.verifyMethodCondition(this, "contains", [1])

the code above doesn't do what we want: to invoke the contains method on the closure. If we leave this groovy will use a reference to the spec at this place.

The bug was caused by the PR #606
The problem with the with expressions is that they are part of spec, with is a method of spock.lang.Specification and should always be invoked on the spec. The PR changed that behavior.
I've raised another PR #782 to exclude the with expressions from being invoked on the closure and leave them being invoked on the spec.

angry-cellophane added a commit to angry-cellophane/spock that referenced this issue Oct 16, 2017

Fix spockframework#778 Nested with expressions are wrapped with Spock…
…Runtime::verifyMethodCondition.

Due to the fix in PR spockframework#606 if there is a method invocation with implicit this inside a with block, SpockRuntime::verifyMethodCondition is invoked on the closure, but not on the this variable, because it this points to the spec. If the with block has a nested with block it is invoked on the closure, which is wrong because it should be invoked on the spec. Additionally the nested with is wrapped with another SpockRuntime::verifyMethodCondition and because Specification::with returns no value SpockRuntime::verifyMethodCondition always fails at this point

leonard84 added a commit to leonard84/spock that referenced this issue Jul 7, 2018

Fix spockframework#778 Nested with expressions are wrapped with Spock…
…Runtime::verifyMethodCondition.

Due to the fix in PR spockframework#606 if there is a method invocation with implicit this inside a with block, SpockRuntime::verifyMethodCondition is invoked on the closure, but not on the this variable, because it this points to the spec. If the with block has a nested with block it is invoked on the closure, which is wrong because it should be invoked on the spec. Additionally the nested with is wrapped with another SpockRuntime::verifyMethodCondition and because Specification::with returns no value SpockRuntime::verifyMethodCondition always fails at this point

@leonard84 leonard84 added the bug label Jul 7, 2018

leonard84 added a commit to leonard84/spock that referenced this issue Jul 20, 2018

Fix spockframework#778 Nested with expressions are wrapped with Spock…
…Runtime::verifyMethodCondition.

Due to the fix in PR spockframework#606 if there is a method invocation with implicit this inside a with block, SpockRuntime::verifyMethodCondition is invoked on the closure, but not on the this variable, because it this points to the spec. If the with block has a nested with block it is invoked on the closure, which is wrong because it should be invoked on the spec. Additionally the nested with is wrapped with another SpockRuntime::verifyMethodCondition and because Specification::with returns no value SpockRuntime::verifyMethodCondition always fails at this point

@leonard84 leonard84 closed this in bc612c0 Aug 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment