Skip to content
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

JsonMinion Should Read ResponseBody Regardless of Expectations #84

Open
akefirad opened this issue Jun 19, 2023 · 5 comments
Open

JsonMinion Should Read ResponseBody Regardless of Expectations #84

akefirad opened this issue Jun 19, 2023 · 5 comments

Comments

@akefirad
Copy link

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants