agorapulse / gru

HTTP Interactions Test Framework
15 stars 4 forks source link

JsonMinion Should Read ResponseBody Regardless of Expectations #84

Open akefirad opened 1 year ago

akefirad commented 1 year ago

Does it make sense?

My use case is, I want to move all expectations out of Gru. Something like this (taken from the documentation):

void 'test it works'() {
    when:                                                                           
        gru.test {
            get '/moons/earth/moon'
            expect {
                // json 'moon.json' <- this!
            }
        }

    then:
        gru.verify()                                                                

    when:
        String responseText = gru.squad.ask(JsonMinion) { responseText }            
    then:
        responseText.contains('moon')     

The above code doesn't work since JsonMinion reads the response body only if there's an expectation set. (I add the minion manually!) To fix the issue I'm adding something like this as an expectation:

    private static class NullContent implements Content {
        @Override
        InputStream load(Client client) {
            return null
        }

        @Override
        boolean isSaveSupported() {
            return false
        }

        @Override
        void save(Client client, InputStream stream) throws IOException {
            throw new UnsupportedOperationException("Saving content is not supported!")
        }
    }

Which seems unnecessary. Is there any reason JsonMinion omits reading response body if there's no responseContent?

Great work BTW!

musketyr commented 1 year ago

hi, reading the response content is a tricky operation. for example when you're using Http client which is based on OkHttp then you get an exception when you try to read the content more than once.

unless very special use cases (such as some auth flow), every interaction should happen within the test or verify method. I think we should provide more Hamcrest matcher style assertions there. these should resole your use case.

musketyr commented 1 year ago

also the same applies as for #83 - responseContent is an expected content. if you need the actual values you should use client.response.text.

akefirad commented 1 year ago

Thanks for the reply. Hm I'm not sure since the documentation says "Use JsonMinion" to get the response body. I understand the purpose of responseContent and fully aware of the issue with reading the body twice (I faced it myself 😬). The problem is more the way JsonMinion works. More specifically; how to force JsonMinion to capture the response body. Right now it does it only if you set an expectation. But what if I don't have expectation (describable using Gru functionality) and need the response body further down in the test. As for doing everything in the test or verify block, unless it's a fundamental requirement (which I don't see it is, please correct me if I'm missing something), that's unnecessary coupling IMHO. Does that make sense? (The same applies to the other issue I reported.)

musketyr commented 1 year ago

As for doing everything in the test or verify block, unless it's a fundamental requirement (which I don't see it is, please correct me if I'm missing something), that's unnecessary coupling IMHO. Does that make sense? (The same applies to the other issue I reported.)

Gru was build to be reusable from the beginning so after calling the verification everything should reset to the default values. Ideally, one should only use verify method but for the reasons how Spock works with mocks it's required to sometimes split the calls into few parts.

I can see your issue with the content capturing. Let's keep the issue open. I will try to figure out something to be able to get the content more easily.

akefirad commented 1 year ago

Sure, let me know if I can help with anything.