Skip to content

test: improve shared helpers #2362

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.

Sign up for GitHub

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

Merged
emadum merged 17 commits into from
May 13, 2020
Merged

test: improve shared helpers #2362

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
90642ba
refactor: improve shared helpers
emadum May 10, 2020
b9055ce
cleanup
emadum May 10, 2020
cf2590b
handle callbacks in withClient
emadum May 11, 2020
4ea6a70
refactor withClient to return test function if not called from it
emadum May 11, 2020
e37232d
remove withClient binds in logger tests
emadum May 12, 2020
bed7aad
add filter function to filterForCommands/filterOutCommands/withMonito…
emadum May 12, 2020
48706c1
add withCollection helper
emadum May 12, 2020
fe3dd10
clean up evg output
emadum May 12, 2020
ae9a5c2
fix for tryNext tests
emadum May 12, 2020
8dea5f7
display eslint version in check:lint script
emadum May 12, 2020
0ebebe3
remove withDb/withCollection helpers
emadum May 12, 2020
d88e609
fix lint error
emadum May 13, 2020
f0fcde8
use typescript syntax for documenting callbacks
emadum May 13, 2020
f894c1a
refactor: use withClient in withMonitoredClient
emadum May 13, 2020
91a7d1e
clean up destructuring in index test
emadum May 13, 2020
23e49b7
fix withMonitoredClient
emadum May 13, 2020
299927f
fix helper tests
emadum May 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .evergreen/install-dependencies.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ NPM_TMP_DIR="${NODE_ARTIFACTS_PATH}/tmp"

# this needs to be explicitly exported for the nvm install below
export NVM_DIR="${NODE_ARTIFACTS_PATH}/nvm"
export XDG_CONFIG_HOME=${NODE_ARTIFACTS_PATH}

# create node artifacts path if needed
mkdir -p ${NODE_ARTIFACTS_PATH}
mkdir -p ${NPM_CACHE_DIR}
mkdir -p "${NPM_TMP_DIR}"

# install Node.js
curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.8/install.sh | bash
curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.35.3/install.sh | bash
[ -s "${NVM_DIR}/nvm.sh" ] && \. "${NVM_DIR}/nvm.sh"
nvm install --lts=${NODE_LTS_NAME}
nvm install --no-progress --lts=${NODE_LTS_NAME}

# setup npm cache in a local directory
cat <<EOT > .npmrc
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"check:atlas": "node test/tools/atlas_connectivity_tests.js",
"check:bench": "node test/benchmarks/driverBench",
"check:coverage": "nyc mocha --timeout 60000 --recursive test/functional test/unit",
"check:lint": "eslint index.js lib test",
"check:lint": "eslint -v && eslint index.js lib test",
"check:test": "mocha --recursive test/functional test/unit",
"check:types": "tsc -p tsconfig.check.json",
"release": "standard-version -i HISTORY.md",
Expand Down
86 changes: 40 additions & 46 deletions test/functional/change_stream.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const assert = require('assert');
const { Transform } = require('stream');
const { MongoError, MongoNetworkError } = require('../../lib/error');
const { setupDatabase, withTempDb, delay } = require('./shared');
const { delay, setupDatabase, withClient } = require('./shared');
const co = require('co');
const mock = require('mongodb-mock-server');
const chai = require('chai');
Expand Down Expand Up @@ -2599,65 +2599,59 @@ describe('Change Streams', function() {
});

describe('tryNext', function() {
function withTemporaryCollectionOnDb(database, testFn) {
return withClient((client, done) => {
const db = client.db(database);
db.createCollection('test', { w: 'majority' }, (err, collection) => {
if (err) return done(err);
testFn(collection, () => db.dropDatabase(done));
});
});
}
it('should return null on single iteration of empty cursor', {
metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } },
test: function() {
return withTempDb(
'testTryNext',
{ w: 'majority' },
this.configuration.newClient(),
db => done => {
const changeStream = db.collection('test').watch();
tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.not.exist;
test: withTemporaryCollectionOnDb('testTryNext', (collection, done) => {
const changeStream = collection.watch();
tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.not.exist;

changeStream.close(done);
});
}
);
}
changeStream.close(done);
});
})
});

it('should iterate a change stream until first empty batch', {
metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } },
test: function() {
return withTempDb(
'testTryNext',
{ w: 'majority' },
this.configuration.newClient(),
db => done => {
const collection = db.collection('test');
const changeStream = collection.watch();
waitForStarted(changeStream, () => {
collection.insertOne({ a: 42 }, err => {
expect(err).to.not.exist;
test: withTemporaryCollectionOnDb('testTryNext', (collection, done) => {
const changeStream = collection.watch();
waitForStarted(changeStream, () => {
collection.insertOne({ a: 42 }, err => {
expect(err).to.not.exist;

collection.insertOne({ b: 24 }, err => {
expect(err).to.not.exist;
});
});
collection.insertOne({ b: 24 }, err => {
expect(err).to.not.exist;
});
});
});

tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.exist;
tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.exist;

tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.exist;
tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.exist;

tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.not.exist;
tryNext(changeStream, (err, doc) => {
expect(err).to.not.exist;
expect(doc).to.not.exist;

changeStream.close(done);
});
});
changeStream.close(done);
});
}
);
}
});
});
})
});
});

Expand Down
28 changes: 13 additions & 15 deletions test/functional/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
var test = require('./shared').assert;
var setupDatabase = require('./shared').setupDatabase;
const expect = require('chai').expect;
const withClient = require('./shared').withClient;
const withMonitoredClient = require('./shared').withMonitoredClient;
const shared = require('./shared');
const { withClient, withMonitoredClient } = shared;

@mbroadst mbroadst May 13, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if we're destructuring anyway it looks like just about all of these come from shared:

const {
  withClient,
  withMonitoredClient,
  assert,
  setupDatabase
} = require('./shared');

@emadum emadum May 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mbroadst I updated a few things since your review, can you take a look when you get a chance? Refactoring withMonitoredClient exposed an issue in withClient, where it was swallowing errors returned by the done(err) callback in tests. I've updated withClient to now throw those errors after cleanup.


describe('Indexes', function() {
before(function() {
Expand Down Expand Up @@ -1208,20 +1208,18 @@ describe('Indexes', function() {
function throwErrorTest(testCommand) {
return {
metadata: { requires: { mongodb: '<4.4' } },
test: function() {
return withClient(this.configuration.newClient(), client => done => {
const db = client.db('test');
const collection = db.collection('commitQuorum');
testCommand(db, collection, (err, result) => {
expect(err).to.exist;
expect(err.message).to.equal(
'`commitQuorum` option for `createIndexes` not supported on servers < 4.4'
);
expect(result).to.not.exist;
done();
});
test: withClient((client, done) => {
const db = client.db('test');
const collection = db.collection('commitQuorum');
testCommand(db, collection, (err, result) => {
expect(err).to.exist;
expect(err.message).to.equal(
'`commitQuorum` option for `createIndexes` not supported on servers < 4.4'
);
expect(result).to.not.exist;
done();
});
}
})
};
}
it(
Expand Down
67 changes: 23 additions & 44 deletions test/functional/logger.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';
var expect = require('chai').expect;
var connectToDb = require('./shared').connectToDb;
const expect = require('chai').expect;
const { withClient } = require('./shared');
const Logger = require('../../lib/logger');

describe('Logger', function() {
Expand Down Expand Up @@ -54,27 +54,20 @@ describe('Logger', function() {
it('should not fail with undefined id', {
metadata: { requires: { topology: ['single'] } },

test: function(done) {
var self = this;

test: function() {
// set a custom logger per http://mongodb.github.io/node-mongodb-native/2.0/tutorials/logging/
Logger.setCurrentLogger(function() {});
Logger.setLevel('debug');

connectToDb('mongodb://localhost:27017/test', self.configuration.db, function(
err,
db,
client
) {
expect(err).to.not.exist;

return withClient('mongodb://localhost:27017/test', (client, done) => {
const db = client.db(this.configuration.db);
// perform any operation that gets logged
db.collection('foo').findOne({}, function(err) {
expect(err).to.not.exist;

// Clean up
Logger.reset();
client.close(done);
done();
});
});
}
Expand All @@ -86,16 +79,9 @@ describe('Logger', function() {
it('should correctly log cursor', {
metadata: { requires: { topology: ['single'] } },

test: function(done) {
var self = this;

connectToDb('mongodb://localhost:27017/test', self.configuration.db, function(
err,
db,
client
) {
expect(err).to.not.exist;

test: function() {
return withClient('mongodb://localhost:27017/test', (client, done) => {
const db = client.db(this.configuration.db);
// Status
var logged = false;

Expand All @@ -120,7 +106,7 @@ describe('Logger', function() {

// Clean up
Logger.reset();
client.close(done);
done();
});
});
}
Expand All @@ -132,35 +118,28 @@ describe('Logger', function() {
it('should pass the logLevel down through the options', {
metadata: { requires: { topology: ['single'] } },

test: function(done) {
var self = this;

test: function() {
Logger.filter('class', ['Cursor']);
var logged = false;

connectToDb(
'mongodb://localhost:27017/test',
self.configuration.db,
{
return withClient('mongodb://localhost:27017/test', (client, done) => {
const db = client.db(this.configuration.db, {
loggerLevel: 'debug',
logger: function() {
logged = true;
}
},
function(err, db, client) {
expect(err).to.not.exist;
});

// perform any operation that gets logged
db.collection('foo').findOne({}, function(err) {
expect(err).to.not.exist;
expect(logged).to.be.true;
// perform any operation that gets logged
db.collection('foo').findOne({}, function(err) {
expect(err).to.not.exist;
expect(logged).to.be.true;

// Clean up
Logger.reset();
client.close(done);
});
}
);
// Clean up
Logger.reset();
done();
});
});
}
});
});
Loading