cjstehno / ersatz

🤖 A simulated HTTP server for testing client code with configurable responses.
https://cjstehno.github.io/ersatz
Apache License 2.0
47 stars 5 forks source link

Variable mistakenly resolved in expectation #107

Closed havenqi closed 5 years ago

havenqi commented 6 years ago

BODY_CONTENT was mistakenly resolved in both request and response body in expectation. I had to use literal String ‘abc’ or local variable to make it work. While the same varialbe can be resolved correctly in chained builder style of coding. In below test spec,

  1. Posting 1: try to replace the body content with BODY_CONTENT and reprodue the issue. ('abc' works)
  2. Posting 2: works if I use a local variable temp to replace BODY_CONTENT
  3. Posting 3: works with chained builder style

    class mockPlay extends Specification {
    private static final String BODY_CONTENT = 'abc'
    @AutoCleanup('stop') private final ErsatzServer server = new ErsatzServer()
    private final OkHttpClient client = new OkHttpClient.Builder().build()
    
    def "Posting 1"() {
        setup:
        server.expectations {
            post('/posting') {
                body 'abc', TEXT_PLAIN               // if BODY_CONTENT is used here, got '404: Not Found' due to failure of matching. Instead 'abc' works
                decoder TEXT_PLAIN, getUtf8String()
    
                responder {
                    body BODY_CONTENT             // if BODY_CONTENT is used her, got bad response: 'Expectations (ErsatzRequest): <GET>, "BODY_CONTENT"'. Instead 'abc' works
                    encoder(TEXT_PLAIN, String) {
                        obj -> obj
                    }
                }
            }
        }.start()
    
        when:
        String value = exec(clientPost('/posting', 'text/plain; charset=utf-8', BODY_CONTENT).build()).body().string()
        then:
        value == BODY_CONTENT
    }
    
    def "Posting 2"() {
        setup:
        def temp =  BODY_CONTENT    // I have a local variable instead of BODY_CONTENT, it works.
        server.expectations {
            post('/posting') {
                body temp, TEXT_PLAIN
                decoder TEXT_PLAIN, getUtf8String()
    
                responder {
                    body temp
                    encoder(TEXT_PLAIN, String) {
                        obj -> obj
                    }
                }
            }
        }.start()
    
        when:
        String value = exec(clientPost('/posting', 'text/plain; charset=utf-8', BODY_CONTENT).build()).body().string()
        then:
        value == BODY_CONTENT
    }
    
    def "Posting 3"() {
        setup:
        server.expectations { // here I used BODY_CONTENT in both request and response, they work!
            post('/posting').body(BODY_CONTENT, TEXT_PLAIN).decoder(TEXT_PLAIN, Decoders.utf8String).responds().body('abc', TEXT_PLAIN)
        }.start()
    
        when:
        String value = exec(clientPost('/posting', 'text/plain; charset=utf-8', BODY_CONTENT).build()).body().string()
    
        then:
        value == BODY_CONTENT
    }
    private Builder clientPost(final String path, final String contentType, final String content) {
        new Builder().post(create(parse(contentType), content)).url("${server.httpUrl}${path}")
    }
    private Response exec(Request req) {
        client.newCall(req).execute()
    }
    }
cjstehno commented 6 years ago

So a bug was reported and fixed in 1.8.1 related to how variables are resolved in the Groovy closures. The closures now resolve the delegate first, which is really the best approach; however, this can cause some odd scoping issues especially with static content.

While this is not ideal, you should be able to use variables in the test method itself, such as:

def 'some test'(){
    setup:
    String bodyContent = BODY_CONTENT
    server.expectations {
        ... use bodyContent in here rather than BODY_CONTENT
    }
}

When I have time I will try to look into ways to resolve this issue so that it works as it did before, but for now, please just use the workaround I mentioned above.

Note: yes, the fluid build approach will work too since it is standard Java, not a Groovy closure.

cjstehno commented 6 years ago

To summarize from the other issue (closed):

When the Groovy closure variable resolution gets confused it does some odd things. The response body DSL method calls the request body object in this case which creates another request expectation.

Some workarounds:

I will see what I can find.

havenqi commented 6 years ago

Thanks for your quick answer. I'll also try java way to confirm if any other odd things occur with closure again.