test(countDocuments): full test coverage for countDocuments

Add tests that were node-specific or missing from spec tests.

Fixes NODE-1559

Author: rweinberger

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);
   });
 }
 

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",

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');