Skip to content
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

Fix several flaky tests (updated) #6824

Merged
merged 10 commits into from
Aug 7, 2018
Merged

Conversation

Fonger
Copy link
Contributor

@Fonger Fonger commented Aug 7, 2018

Fix test flakes to increase CI pass rate

1. Fix flaky test case for #4014

  • Ensure 2dsphere index is created before the query is executed

  • Specify spherical: true to use 2dsphere index instead of 2d index

    I don't know why it passes the test without this patch sometimes. This appears to be a bug and should fail every time without this fix.

    Mocha Output

    1) document
         bug fixes
           single embedded docs with $near (gh-4014):
           Uncaught MongoError: error processing query:
             ns=mongoose_test.gh4014
             Tree: GEONEAR  field=geo maxdist=1.79769e+308 isNearSphere=0
       Sort: {}
       Proj: {}
       planner returned error: unable to find index for $geoNear query
         at queryCallback (node_modules/mongodb-core/lib/cursor.js:247:25)
         at node_modules/mongodb-core/lib/connection/pool.js:531:18
         at process._tickCallback (internal/process/next_tick.js:176:11)
    

    See build failure on travis ci - [1] [2] [3]

2. Fix flaky test for #6271 (EDIT: refactored)

  • By ensuring the update request with callback is executed

    Mocha Output

    1) Query
           bug fixes
             consistently return query when callback specified (gh-6271):
          AssertionError [ERR_ASSERTION]: 1 == 2
          + expected - actual
          -1
          +2
          
          at Decorator._callFunc (node_modules/empower-core/lib/decorator.js:110:20)
          at Decorator.concreteAssert (node_modules/empower-core/lib/decorator.js:103:17)
          at Function.decoratedAssert [as equal] (node_modules/empower-core/lib/decorate.js:51:30)
          at test/query.test.js:2457:16
          at Generator.next (<anonymous>)
          at onFulfilled (node_modules/co/index.js:65:19)
          at <anonymous>
          at process._tickCallback (internal/process/next_tick.js:188:7)
    

    See build failure on travis-ci

3. Add delay for test case in #4027

  • Make sure driver has enough time to give up reconnection and emit reconnectFailed event.

    Mocha Output

      1) connections:
           openUri (gh-5304)
             connection events
               reconnectFailed (gh-4027):
    
          AssertionError [ERR_ASSERTION]: 0 == 1
          + expected - actual
    
          -0
          +1
    
          at Decorator._callFunc (node_modules/empower-core/lib/decorator.js:110:20)
          at Decorator.concreteAssert (node_modules/empower-core/lib/decorator.js:103:17)
          at Function.decoratedAssert [as equal] (node_modules/empower-core/lib/decorate.js:51:30)
          at test/connection.test.js:217:20
    

Miscellaneous

1. Fix duplicate test queries introduced in #6016

2. Update MongoDB configuration on Travis

@Fonger
Copy link
Contributor Author

Fonger commented Aug 7, 2018

@vkarpov15 updated based on your review

I've tested several times on my local development environment and on my travis-ci.
It seems that all of the flaky tests are fixed after this pull request except for timeout error.

Maybe we can write a script to make travis-ci automatically retry the test if the failure reason is timeout?

@Fonger Fonger changed the title Fix several flaky tests Fix several flaky tests (updated) Aug 7, 2018
@Fonger Fonger force-pushed the improve-ci-test branch 4 times, most recently from 65c2345 to 783b660 Compare August 7, 2018 18:15
assert.ifError(error);
done();
});
MyModel.on('index', function(err) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general its better to use MyModel.init().then(() => {}) instead of MyModel.on('index'), because with the latter if you register the event listener after the index build is finished, you'll never get a callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it really makes sense! There are similar test codes like this elsewhere and I will try to update it in future fix to make the test more robust.

Although index takes time to build, if we use some sort of index cache in mongoose in the future, or MyModel.on('index') is not placed right after MyModel.model() in the same event loop, that's a potential issue.

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thanks! This should make Travis much more reliable.

@vkarpov15 vkarpov15 added this to the 5.2.8 milestone Aug 7, 2018
@vkarpov15 vkarpov15 merged commit 2e71fea into Automattic:master Aug 7, 2018
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.

2 participants