-
Notifications
You must be signed in to change notification settings - Fork 69
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
test: introduce tests for @requires #704
Conversation
f867974
to
017a5c3
Compare
Hi @clayne11 Would you mind rebasing? If things are still failing, we'll investigate. Thank you for the contribution! |
017a5c3
to
fafc4b9
Compare
Currently it does not work as expected.
fafc4b9
to
c78416c
Compare
Rebased — tests are still failing.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
This is not stale, please don't close it. |
@clayne11 Hi Curtis, Apologies for my lack of updates. We know what the issue is, and we're working on some fixes. We're still interested in the PR and don't plan to close this. Thanks for your patience! |
Hi @clayne11 we have merged a fix for an issue preventing from running tests I split requires tests into 2 separate tests and fixed requires resolvers |
Motivation and Context
The Cosmo router ostensibly supports the
@requires
directive, however it doesn't seem to work properly. Added tests to router-tests to show the expected behavior.These tests currently fail since the router logic seems to be incorrect. This is both true for @requires directives with flat fields as well as ones with nested selection sets, although each of these lead to different error patterns.