adding bailout for query instance and test #13

ghost · 2018-04-21T14:45:40Z

The details of why I made these changes are in Automattic/mongoose#6375 (comment)

I'm not super familiar with babel or tape, so I won't be offended if you think my changes could be done differently. I'm happy to make any adjustments that you feel it needs.

I hope this helps :)

ghost · 2018-04-21T14:48:24Z

I just noticed that when the tests run on mongoose 4, the array that is returned is ordered backwards. I'll have to give this some thought.

just a first pass without refactoring, but something along these lines ought to get it done:

  let found = await Test.findOne({ name: doc.name })
  if (/^4/.test(mongoose.version)) {
    t.same(_.toArray(found.keywords), ['rock', 'test'], 'should work with findone by name')
  } else if (/^5/.test(mongoose.version)){
    t.same(_.toArray(found.keywords), ['test', 'rock'], 'should work with findone by name')
  }

refactored to:

  const found = await Test.findOne({ name: doc.name })
  const arr = ['test', 'rock']
  const major = mongoose.version.charAt(0)
  if(major < 5) {
    arr.sort()
  }

  t.same(_.toArray(found.keywords), arr, 'should work with findone by name')

diegohaz · 2018-04-21T17:08:48Z

Hi, @lineus. Thank you so much for your help. I think the failing tests on CI is due to the use of async/await not being supported by Node < 7.

Could you update .travis.yml to run only v7? (just remove the other versions).

ghost · 2018-04-21T17:30:06Z

I removed the async/await and used straight up promises instead. If you'd prefer me to update the travis.yml I can do that instead, but this way I'm not changing your travis setup.

diegohaz · 2018-04-21T17:35:22Z

Great. I think we have finally fixed it. Thank you, @lineus.

ghost · 2018-04-21T17:35:30Z

You're very welcome by the way 😀 This was fun to work on! It was neat to play around in your plugin too, I learned several new things from it.

vkarpov15 · 2018-04-22T18:42:18Z

Thanks for fixing this! Don't hesitate to reach out if you have any other issues @diegohaz , we're happy to help 👍

adding bailout for query instance and test

aad60bc

Kev added 3 commits April 21, 2018 11:08

updated test to work with mongoose 4 and 5

948fbe1

fixing spacing issue on conditional test for major version

0b7d559

changing import statement to be more specific

35cc618

remove async/await, use plain old promises instead

d97f26d

diegohaz merged commit 4e2572f into diegohaz:master Apr 21, 2018

diegohaz mentioned this pull request Apr 21, 2018

Broken in Mongoose 5.x.x #12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding bailout for query instance and test #13

adding bailout for query instance and test #13

adding bailout for query instance and test #13

adding bailout for query instance and test #13

Conversation