Skip to content

Conversation

@draylang
Copy link

Tests were failing due to uncertain key order in results. I made a shim that is to be used during testing only that fixes most of the cases. There are 2 instances that I added sorting to the error output in the source file but these should be low impact changes.

@khrt
Copy link
Owner

khrt commented Sep 13, 2017

Hi @draylang, you made a huge work and I really appreciate it, but I don't really like the idea of sorting hashes.

If you take a look at the tests you can see that in similar cases I'm using cmp_deeply from Test::Deep. Here are the examples.

If you would change your PR to use cmp_deeply instead of sort I'll be glad to merge it.

@draylang
Copy link
Author

The tests are failing on a string match, not a deep structure comparison. Example from execute-variables.t:

is_deeply execute($schema, $ast), {
data => {
fieldWithObjectInput => encode_json({ a => 'foo', b => ['bar'], c => 'baz' }),
}
};

The value in fieldWithObjectInput is a string. A json string that is composed during each run on both the result and the test. The order in which the elements are composed into this json string is not guaranteed.

Do your tests currently run reliably without failing? They don't for me and the failures are randomly changing between runs.

Another fix would be to dispense with the json encoding altogether and compare structured data as you suggest with cmp_deeply. That would still require hooking or wrapping the graphql and execute methods and parsing the result as well as needing to touch a lot of test code to remove the json encoding for each test. I was aiming for a solution with the least number of changes.

@khrt
Copy link
Owner

khrt commented Sep 13, 2017

Tests are passing for me on Perl 5.16.3 and 5.10.1. But I aware some of them are unstable depends on Perl build parameters.

@mohawk2
Copy link

mohawk2 commented Oct 16, 2017

Hi guys (hi Artur!), if you'd like to then I'd really appreciate any help you might want to give to the other graphql-perl: https://github.com/graphql-perl/graphql-perl - https://metacpan.org/pod/GraphQL on CPAN.

Or, if you're interested:

Next phases of implementation will be:

  • make it handle promises using the Future module
  • build schemas from ASTs (the parser already handles this)
  • make a graphql-perl tutorial based heavily on the graphql-js one

Let me know your thoughts!

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

Successfully merging this pull request may close these issues.

3 participants