mattkrick/meatier

Failling tests on example

Closed this issue · 17 comments

Hi,
When you launch tests just after install:
2 tests failed

  1. createUser:success
    AssertionError: { data:
    authToken: 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ImI3ZjIyODIxLTY
    callee3$ (src/server/graphql/models/User/tests/userMutation-tests.js:226:15)
  2. createUser:alreadyexists
    AssertionError: { data:
    authToken: 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6Ijk4YjM5NjZiLTd
    callee4$ (src/server/graphql/models/User/tests/userMutation-tests.js:326:15)

After some investigation it seems that user.strategies.local are not equal but I don't know how to fix the problem...

@yatsu fix this in his fork with the following diff;

-  t.same(actual, expected)
+  t.same(actual.data.newUser.user.id, expected.data.newUser.user.id);
+  t.same(actual.data.newUser.user.email, expected.data.newUser.user.email);
+  t.same(actual.data.newUser.user.createdAt, expected.data.newUser.user.createdAt);
yatsu commented

@gouroujo

It is just a quick fix to pass the test.
I don't think it is a right solution.

yeah, it's bizarre, i think there's a problem with the AVA deep equal algo in t.same but i haven't had time to investigate. If you find out anything more let me know or open up an issue over there

So in AVA t.true means val === true and t.same means deep strict equal, this is different from what people that come from tape or tap might expect since tape's t.same means deep non-strict equal and t.ok is !!val

The first test failure occurred in createUser:success due to an trailing error being attached to the actual result:

"errors": [
  {
    "message": "property.call is not a function",
    "originalError": {}
  }

otherwise actual and expected outputs are identical minus the added error in actual:

{
  "data": {
  "newUser": {
    "user": {
      "id": "0f6f6e07-d36f-42fc-9bd4-b6b54781ade4",
        "email": "createuser:success@createuser:success",
        "createdAt": null,
        "updatedAt": null,
        "strategies": {
        "local": {
          "isVerified": false,
            "password": null,
            "verifiedEmailToken": null,
            "resetToken": null
        },
        "google": null
      }
    },
    "authToken": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6IjBmNmY2ZTA3LWQzNmYtNDJmYy05YmQ0LWI2YjU0NzgxYWRlNCIsImlhdCI6MTQ1NzU4Nzc4MywiZXhwIjoxNDU4MTkyNTgzfQ.z2PWWAekJ5RccX6lYvtUZQ3ImOPIlsvxGILlrrufAKs"
  }
},
  "errors": [
  {
    "message": "property.call is not a function",
    "originalError": {}
  }
]
}

vs

expected:

{
  "data": {
  "newUser": {
    "user": {
      "id": "0f6f6e07-d36f-42fc-9bd4-b6b54781ade4",
        "email": "createuser:success@createuser:success",
        "createdAt": null,
        "updatedAt": null,
        "strategies": {
        "local": {
          "isVerified": false,
            "password": null,
            "verifiedEmailToken": null,
            "resetToken": null
        },
        "google": null
      }
    },
    "authToken": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6IjBmNmY2ZTA3LWQzNmYtNDJmYy05YmQ0LWI2YjU0NzgxYWRlNCIsImlhdCI6MTQ1NzU4Nzc4MywiZXhwIjoxNDU4MTkyNTgzfQ.z2PWWAekJ5RccX6lYvtUZQ3ImOPIlsvxGILlrrufAKs"
  }
}
}

FYI yatsu fix is not bad way to circumvent what is most likely an error cause by updates to one of the modules used in the test. In a way running more concise test while not fixing the underlining issue being property.call is not a function is not optimal but it focuses on comparing apples to apples and all things equal, it proves that build works, albite artifacts are present in its current state.

So to pass both failing test yatsu replaced t.same(actual, expected) with:

t.same(actual.data.newUser.user.id, expected.data.newUser.user.id);
  t.same(actual.data.newUser.user.email, expected.data.newUser.user.email);
  t.same(actual.data.newUser.user.createdAt, expected.data.newUser.user.createdAt);

Agree, the more declarative approach is nicer, just longer to write. I'll
try to fix it up today. I'm still curious why the deep equal is now
failing...

On Thu, Mar 10, 2016, 12:16 AM Bartek Kus notifications@github.com wrote:

FYI yatsu fix is not bad way to circumvent what is most likely an error
cause by updates to one of the modules used in the test. In a way running
more concise test while not fixing the underlining issue being property.call
is not a function is not optimal but it focuses on comparing apples to
apples and all things equal, it proves that build works, albite artifacts
are present in its current state.

So to pass both failing test yatsu replaced t.same(actual, expected) with:

t.same(actual.data.newUser.user.id, expected.data.newUser.user.id);
t.same(actual.data.newUser.user.email, expected.data.newUser.user.email);
t.same(actual.data.newUser.user.createdAt, expected.data.newUser.user.createdAt);


Reply to this email directly or view it on GitHub
#103 (comment).

Root cause is that both objects have to share the same constructor to be equal...

fixed by avajs/ava#640. If this is an immediate concern you can pull the master branch of AVA from github, otherwise the npm package will be released this week with the fix.

Well done @mattkrick and on behalf of the meatier/AVA community, thank you good sir!

If we reset the db before running the test suite we get the following graphql error which is causing the inequality referenced above:

TypeError: property.call is not a function
    at defaultResolveFn (./node_modules/graphql/execution/execute.js:600:54)
    at resolveOrError (./node_modules/graphql/execution/execute.js:426:12)
    at resolveField (./node_modules/graphql/execution/execute.js:417:16)
    at ./node_modules/graphql/execution/execute.js:235:18
    at Array.reduce (native)
    at executeFields (./node_modules/graphql/execution/execute.js:233:43)
    at completeValue (./node_modules/graphql/execution/execute.js:585:10)
    at completeValueCatchingError (./node_modules/graphql/execution/execute.js:446:21)
    at resolveField (./node_modules/graphql/execution/execute.js:419:10)
    at ./node_modules/graphql/execution/execute.js:235:18
    at Array.reduce (native)
    at executeFields (./node_modules/graphql/execution/execute.js:233:43)
    at completeValue (./node_modules/graphql/execution/execute.js:585:10)
    at ./node_modules/graphql/execution/execute.js:490:14
    at run (./node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:89:22)
    at ./node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:102:28
    at flush (./node_modules/babel-polyfill/node_modules/core-js/modules/_microtask.js:14:5)
    at _combinedTickCallback (node.js:370:9)
    at process._tickCallback (node.js:401:11)

@wenzowski sometimes i noticed that the first time i run a test graphQL returns an error object, even if there is no error. then, i run it again & it works as expected. is this in reference to that?

Yes, @mattkrick this is that.
I'm hoping that if this turns out to be an upstream issue we can report it.
Haven't spent enough time digging through the graphql reference implementation to know what's going on offhand.

The final issue appears to be caused by call becoming unbound in defaultResolveFn

that's wild, thanks for finding it! are you use it's the defaultResolveFn that is in the graphQL package & not the one that is used in meatier? I had to copy it over to enable auth & i figure it's much more likely that my shoddy code is to blame instead of graphQL.

Absolutely sure it's becoming unbound in the default graphql function at defaultResolveFn (/home/ubuntu/meatier/node_modules/graphql/execution/execute.js:600:54) because the upstream pr I submitted fixes it...though I have no idea what is causing call to become unbound offhand.

oh man, great find! i'm stumped on that one as well, it'd be fun to dive in this weekend & see why...