From c25c5197f3e1c6c26899898708fc142c41245cbd Mon Sep 17 00:00:00 2001 From: Rebecca Weinberger Date: Wed, 18 Jul 2018 12:18:40 -0400 Subject: [PATCH] test(countDocuments): full test coverage for countDocuments Add tests that were node-specific or missing from spec tests. Fixes NODE-1559 --- lib/operations/collection_ops.js | 7 +- package.json | 1 + test/functional/collection_tests.js | 166 +++++++++++++++++++++++++++- 3 files changed, 168 insertions(+), 6 deletions(-) diff --git a/lib/operations/collection_ops.js b/lib/operations/collection_ops.js index e317f982b6..7d43d19dc8 100644 --- a/lib/operations/collection_ops.js +++ b/lib/operations/collection_ops.js @@ -230,12 +230,9 @@ function countDocuments(coll, query, options, callback) { delete options.limit; delete options.skip; - coll.aggregate(pipeline, options, (err, result) => { + coll.aggregate(pipeline, options).toArray((err, docs) => { if (err) return handleCallback(callback, err); - result.toArray((err, docs) => { - if (err) return handleCallback(err); - handleCallback(callback, null, docs.length ? docs[0].n : 0); - }); + handleCallback(callback, null, docs.length ? docs[0].n : 0); }); } diff --git a/package.json b/package.json index afcaa6f7fb..3487655558 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "prettier": "~1.12.0", "semver": "5.4.1", "sinon": "^4.3.0", + "sinon-chai": "^3.2.0", "worker-farm": "^1.5.0" }, "author": "Christian Kvalheim", diff --git a/test/functional/collection_tests.js b/test/functional/collection_tests.js index 18ea41d6ac..f21c468ac2 100644 --- a/test/functional/collection_tests.js +++ b/test/functional/collection_tests.js @@ -1,8 +1,12 @@ 'use strict'; const test = require('./shared').assert; const setupDatabase = require('./shared').setupDatabase; -const expect = require('chai').expect; +const chai = require('chai'); +const expect = chai.expect; const MongoClient = require('../..').MongoClient; +const sinonChai = require('sinon-chai'); +const mock = require('mongodb-mock-server'); +chai.use(sinonChai); describe('Collection', function() { before(function() { @@ -1616,6 +1620,166 @@ describe('Collection', function() { }); }); + it('countDocuments should return Promise that resolves when no callback passed', function(done) { + const configuration = this.configuration; + const client = new MongoClient(configuration.url(), { w: 1 }); + + client.connect(function(err, client) { + const db = client.db(configuration.db); + const collection = db.collection('countDoc_return_promise'); + const docsPromise = collection.countDocuments(); + const close = e => client.close(() => done(e)); + + expect(docsPromise).to.exist.and.to.be.an.instanceof(collection.s.promiseLibrary); + + docsPromise + .then(result => expect(result).to.equal(0)) + .then(() => close()) + .catch(e => close(e)); + }); + }); + + it('countDocuments should not return a promise if callback given', function(done) { + const configuration = this.configuration; + const client = new MongoClient(configuration.url(), { w: 1 }); + + client.connect(function(err, client) { + const db = client.db(configuration.db); + const collection = db.collection('countDoc_no_promise'); + const close = e => client.close(() => done(e)); + + const notPromise = collection.countDocuments({ a: 1 }, function() { + expect(notPromise).to.be.undefined; + close(); + }); + }); + }); + + it('countDocuments should correctly call the given callback', function(done) { + const configuration = this.configuration; + const client = new MongoClient(configuration.url(), { w: 1 }); + + client.connect(function(err, client) { + const db = client.db(configuration.db); + const collection = db.collection('countDoc_callback'); + const docs = [{ a: 1 }, { a: 2 }]; + const close = e => client.close(() => done(e)); + + collection.insertMany(docs).then(() => + collection.countDocuments({ a: 1 }, (err, data) => { + expect(data).to.equal(1); + close(err); + }) + ); + }); + }); + + describe('countDocuments with mock server', function() { + let server; + + beforeEach(() => { + return mock.createServer().then(s => { + server = s; + }); + }); + + afterEach(() => mock.cleanup()); + + function testCountDocMock(config, done) { + const client = new MongoClient(`mongodb://${server.uri()}/test`); + const close = e => client.close(() => done(e)); + + server.setMessageHandler(request => { + const doc = request.document; + if (doc.aggregate) { + try { + config.replyHandler(doc); + request.reply(config.reply); + } catch (e) { + close(e); + } + } + + if (doc.ismaster) { + request.reply(Object.assign({}, mock.DEFAULT_ISMASTER)); + } else if (doc.endSessions) { + request.reply({ ok: 1 }); + } + }); + + client.connect(function(err, client) { + const db = client.db('test'); + const collection = db.collection('countDoc_mock'); + + config.executeCountDocuments(collection, close); + }); + } + + it('countDocuments should return appropriate error if aggregation fails with callback given', function(done) { + const replyHandler = () => {}; + const executeCountDocuments = (collection, close) => { + collection.countDocuments(err => { + expect(err).to.exist; + expect(err.errmsg).to.equal('aggregation error - callback'); + close(); + }); + }; + + testCountDocMock( + { + replyHandler, + executeCountDocuments, + reply: { ok: 0, errmsg: 'aggregation error - callback' } + }, + done + ); + }); + + it('countDocuments should error if aggregation fails using Promises', function(done) { + const replyHandler = () => {}; + const executeCountDocuments = (collection, close) => { + collection + .countDocuments() + .then(() => expect(false).to.equal(true)) // should never get here; error should be caught + .catch(e => { + expect(e.errmsg).to.equal('aggregation error - promise'); + close(); + }); + }; + + testCountDocMock( + { + replyHandler, + executeCountDocuments, + reply: { ok: 0, errmsg: 'aggregation error - promise' } + }, + done + ); + }); + + it('countDocuments pipeline should be correct with skip and limit applied', function(done) { + const replyHandler = doc => { + expect(doc.pipeline).to.deep.include({ $skip: 1 }); + expect(doc.pipeline).to.deep.include({ $limit: 1 }); + }; + const executeCountDocuments = (collection, close) => { + collection.countDocuments({}, { limit: 1, skip: 1 }, err => { + expect(err).to.not.exist; + close(); + }); + }; + + testCountDocMock( + { + replyHandler, + executeCountDocuments, + reply: { ok: 1 } + }, + done + ); + }); + }); + describe('Retryable Writes on bulk ops', function() { const MongoClient = require('../../lib/mongo_client');