[Feat] Return model as original property on Response about lighthouse HOT 12 CLOSED

@TGNThump it could be a second method called executeAsResponse or you could just call getContent on the Response object. The point is we can't get the data before it gets manipulated which makes testing a lot harder.

from lighthouse.

Hmm. I'm not sure I like this idea, purely because it makes it hard to use the library in cases where you aren't parsing a response object, such as loading a query from another source (DB) or for subscriptions, where you aren't going to be returning a response object.

from lighthouse.

@olivernybroe I personally like the executeAsResponse idea. If there's no PR I'll add it this weekend but thanks for the feedback!!

from lighthouse.

@chrissm79 Sounds great! Looked into it, but tbh. I have no idea how to implement it.

from lighthouse.

Are you still interested in doing something like this? I think the tests work fine as they are, and i am not too big a fan of adding methods just for the sake of testing.

from lighthouse.

@spawnia I think it is really needed. Writing tests right now in Lighthouse isn't that nice as I can only check the returning data, not the actual data, which is what I am testing for in that particular test.

I think it is worth adding a method just for the sake of testing.

from lighthouse.

What is it that you are actually trying to test? For an integration test, we should definitely test the public interface. If it is a unit test, maybe we do not even need to execute the query fully?

from lighthouse.

@spawnia Sometimes I have a complex data definition in graphql. So I want to make sure that the data returned is the correct model. I am not interested in what fields I have defined in my SDL. I am interested in what is received before the data is transformed.

from lighthouse.

Seems like that can be achieved by calling the resolve function directly. Should this prove to be insufficient, you can add some utility functions to TestCase.php to help with such tests.

from lighthouse.

@olivernybroe Have you done anything on this? I would like to avoid adding complexity to the resolution itself, so if you can achieve what you need by adding a helper for testing, i would be up for including it as part of the test suite.

from lighthouse.

@spawnia haven't done anything for this.

from lighthouse.

I think this is a very specific use case for testing, and can be worked around. I think the public API should be kept small, and we can not really control the final execution result without reaching into the GraphQL core library.

Closing this for now, but am open for PR's that improve testability.

from lighthouse.