Spring-cloud-contract: execute() in response's test() works incorrectly

Created on 24 Apr 2018  路  9Comments  路  Source: spring-cloud/spring-cloud-contract

Hello, I have encountered an issue where a test(execute(...)) in the response generates incorrect tests. I expect it to work based on the documentation (https://cloud.spring.io/spring-cloud-contract/single/spring-cloud-contract.html#_regular_expressions).

Minimal example:

Contract.make {
    request {
        method 'GET'
        urlPath '/whatever'
        headers {
            header 'My-UUID': value(test(execute('property("my-uuid")')), stub('76c53386-ad9b-11e6-92dc-0370ae47c3b2'))
        }
    }
    response {
        status 200
        headers {
            header 'My-UUID': value(test(execute('property("my-uuid")')), stub('76c53386-ad9b-11e6-92dc-0370ae47c3b2'))
        }
    }
}

where property(...) would be a method in my base class.

The Spock test I get is:

def validate_bug() throws Exception {
    given:
        def request = given()
                .header("My-UUID", property("my-uuid"))

    when:
        def response = given().spec(request)
                .get("/whatever")

    then:
        response.statusCode == 200
        property("my-uuid")
}

I'd expect the last line to be response.header('My-UUID') == property("my-uuid") instead.

I'm generating tests using the spring-cloud-contract Gradle plugin version 2.0.0.RC1. The issue is the same on 1.2.4.RELEASE.

invalid

Most helpful comment

@kszafran @TYsewyn After the following bug was registered: https://github.com/spring-cloud/spring-cloud-contract/issues/728 we have revisited this issue with @marcingrzejszczak and have reached the conclusion, that the behaviour described here is, in fact correct.

The execute(...) method should actually execute exactly whatever is passed there and not do anything more - that makes sense from the DSL level; @kszafran if you want to add verification, you should add this to the execution methods yourself, like so:

Contract.make {
    request {
       // ...
        headers {
            header 'My-UUID': value(test(execute('property("my-uuid")')), stub('76c53386-ad9b-11e6-92dc-0370ae47c3b2'))
        }
    }
    response {
       // ...
        headers {
            header 'My-UUID': value(test(execute('assertProperty($it)')), stub('76c53386-ad9b-11e6-92dc-0370ae47c3b2'))
        }
    }
}

where aasertProperty() would be a method that verifies that the property is correct.
Therefore, we will be reverting this bug fix, as it's not really a bug. Please adjust your code.

All 9 comments

Wow, that's an interesting bug. Obviously, that's wrong. Thanks for reporting! Can you check if the same problem is there with JUnit tests?

Here's the JUnit version (same issue):

@Test
public void validate_bug() throws Exception {
    // given:
        RequestSpecification request = given()
                .header("My-UUID", property("my-uuid"));

    // when:
        Response response = given().spec(request)
                .get("/whatever");

    // then:
        assertThat(response.statusCode()).isEqualTo(200);
        property("my-uuid");
}

For now I can use a workaround like: test(execute('response.header("My-UUID") == property("my-uuid")'), though it's a bit ugly.

It's not ugly, it's beautiful ;)

Jokes aside, it's good that you have a workaround. We'll try to address this bug asap.

Thanks. I hope you can fix this before 2.0.0 is released.

@Kaef can you verify your contracts against 2.0.0.BUILD-SNAPSHOT if it does what you expect?
Please let us know if there is still a problem with this feature, and do not hesitate to file an issue if you encounter some other weird behaviour! :)

I just tried building our tests with 2.0.0.BUILD-SNAPSHOT and it seems to work just fine. Thanks!

@kszafran @TYsewyn After the following bug was registered: https://github.com/spring-cloud/spring-cloud-contract/issues/728 we have revisited this issue with @marcingrzejszczak and have reached the conclusion, that the behaviour described here is, in fact correct.

The execute(...) method should actually execute exactly whatever is passed there and not do anything more - that makes sense from the DSL level; @kszafran if you want to add verification, you should add this to the execution methods yourself, like so:

Contract.make {
    request {
       // ...
        headers {
            header 'My-UUID': value(test(execute('property("my-uuid")')), stub('76c53386-ad9b-11e6-92dc-0370ae47c3b2'))
        }
    }
    response {
       // ...
        headers {
            header 'My-UUID': value(test(execute('assertProperty($it)')), stub('76c53386-ad9b-11e6-92dc-0370ae47c3b2'))
        }
    }
}

where aasertProperty() would be a method that verifies that the property is correct.
Therefore, we will be reverting this bug fix, as it's not really a bug. Please adjust your code.

Thank you for the info. @mzielinski, @llesiak you might be interested in this change.

Approach suggested by @OlgaMaciaszek works OK. (Tested on 2.1.0.M1)
I've just had to make slight alterations:
assertProperty($it) -> assertProperty($it, "my-uuid")
I prefer to invoke property("my-uuid") once more in assertProperty method as I need to know input value to determine if response is correct.

Was this page helpful?
0 / 5 - 0 ratings