oslabs-beta / GraphQL-Gate

A GraphQL rate limiting library with query complexity analysisfor Node.js
http://graphqlgate.io
MIT License
57 stars 2 forks source link

Updated fragment tests #75

Closed shalarewicz closed 2 years ago

shalarewicz commented 2 years ago

Summary

This updates the test suite for queries using fragments. These tests test for the following:

The test also cleans up function mocks in the complexity suite to avoid mocking incorrect return values when tests are run in parallel.

Type of Change

Issues

Evidence

shalarewicz commented 2 years ago

@evanmcneely Do you think the complexity analysis algo should throw an error for unhandled node cases during development? This could help us catch cases we haven't considered. I'm thinking of the case where the test 'with fragments...that have a complexity of zero' is passing before fragments have been implemented?

evanmcneely commented 2 years ago

@evanmcneely Do you think the complexity analysis algo should throw an error for unhandled node cases during development? This could help us catch cases we haven't considered. I'm thinking of the case where the test 'with fragments...that have a complexity of zero' is passing before fragments have been implemented?

haha, while in development it's probably not a big deal but if there's anything we don't get done before "launch" then ya we probably should. It wouldn't be hard because you could throw on error in the else statement wherever there are cases we aren't handling. If something happens we don't expect throw an error.

This might also be a good way to lay out the rest of the node functions and flow of the calls before implementing the logic.

shalarewicz commented 2 years ago

@evanmcneely Do you think the complexity analysis algo should throw an error for unhandled node cases during development? This could help us catch cases we haven't considered. I'm thinking of the case where the test 'with fragments...that have a complexity of zero' is passing before fragments have been implemented?

haha, while in development it's probably not a big deal but if there's anything we don't get done before "launch" then ya we probably should. It wouldn't be hard because you could throw on error in the else statement wherever there are cases we aren't handling. If something happens we don't expect throw an error.

This might also be a good way to lay out the rest of the node functions and flow of the calls before implementing the logic.

Will add these errors along with the implementation of #44

shalarewicz commented 2 years ago

Good work. Lot's of edge cases I didn't know about. Approve with some things to check.

In implementation, I think you'll run into some problems when looking in the typeWeightsObject for a union and not seeing what it is a union of in order to find the fields you need to get weights of. I wouldn't be surprised if you have to refactor the typeWeightsObject to have that information. We'll only find out when actually doing the work.

Agreed, see my comments on #44. Current idea is just to look for common fields when we parse the schema and add these to both the Union type SearchResult and the individual types Droid and Human.