test(NODE-3860): improve skipReason reporting for disabled 'auth' tes…

…ts (#3137)
mongodb · Feb 10, 2022 · 9242de5 · 9242de5
1 parent 46d5821
commit 9242de5
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 49 deletions.
diff --git a/test/integration/causal-consistency/causal_consistency.prose.test.js b/test/integration/causal-consistency/causal_consistency.prose.test.js
@@ -4,6 +4,7 @@ const { LEGACY_HELLO_COMMAND } = require('../../../src/constants');
 
 const { setupDatabase } = require('../shared');
 const { expect } = require('chai');
+const { skipBrokenAuthTestBeforeEachHook } = require('../../tools/runner/hooks/configuration');
 
 const ignoredCommands = [LEGACY_HELLO_COMMAND, 'endSessions'];
 const test = { commands: { started: [], succeeded: [] } };
@@ -14,6 +15,18 @@ describe('Causal Consistency - prose tests', function () {
     return setupDatabase(this.configuration);
   });
 
+  beforeEach(
+    skipBrokenAuthTestBeforeEachHook({
+      skippedTests: [
+        '2. The first read in a causally consistent session must not send afterClusterTime to the server',
+        'case: successful read with causal consistency',
+        'case: second operation is findOne',
+        'case: successful insert',
+        '6. A read operation in a ClientSession that is not causally consistent should not include the afterClusterTime parameter in the command sent to the server'
+      ]
+    })
+  );
+
   beforeEach(function () {
     test.commands = { started: [], succeeded: [] };
     test.client = this.configuration.newClient({ w: 1 }, { maxPoolSize: 1, monitorCommands: true });
@@ -29,7 +42,7 @@ describe('Causal Consistency - prose tests', function () {
   });
 
   afterEach(() => {
-    return test.client.close();
+    return test.client ? test.client.close() : Promise.resolve();
   });
 
   it(
@@ -42,10 +55,9 @@ describe('Causal Consistency - prose tests', function () {
      */
     {
       metadata: {
-        requires: { topology: ['replicaset', 'sharded'], auth: 'disabled' },
+        requires: { topology: ['replicaset', 'sharded'] },
         // Skipping session leak tests b/c these are explicit sessions
-        sessions: { skipLeakTests: true },
-        skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+        sessions: { skipLeakTests: true }
       },
 
       test: function () {
@@ -81,10 +93,9 @@ describe('Causal Consistency - prose tests', function () {
 
       it('case: successful read with causal consistency', {
         metadata: {
-          requires: { topology: ['replicaset', 'sharded'], auth: 'disabled' },
+          requires: { topology: ['replicaset', 'sharded'] },
           // Skipping session leak tests b/c these are explicit sessions
-          sessions: { skipLeakTests: true },
-          skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+          sessions: { skipLeakTests: true }
         },
 
         test: function () {
@@ -120,10 +131,9 @@ describe('Causal Consistency - prose tests', function () {
     () => {
       it('case: second operation is findOne', {
         metadata: {
-          requires: { topology: ['replicaset', 'sharded'], auth: 'disabled' },
+          requires: { topology: ['replicaset', 'sharded'] },
           // Skipping session leak tests b/c these are explicit sessions
-          sessions: { skipLeakTests: true },
-          skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+          sessions: { skipLeakTests: true }
         },
 
         test: function () {
@@ -166,10 +176,9 @@ describe('Causal Consistency - prose tests', function () {
     () => {
       it('case: successful insert', {
         metadata: {
-          requires: { topology: ['replicaset', 'sharded'], auth: 'disabled' },
+          requires: { topology: ['replicaset', 'sharded'] },
           // Skipping session leak tests b/c these are explicit sessions
-          sessions: { skipLeakTests: true },
-          skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+          sessions: { skipLeakTests: true }
         },
 
         test: function () {
@@ -207,10 +216,9 @@ describe('Causal Consistency - prose tests', function () {
      */
     {
       metadata: {
-        requires: { topology: ['replicaset', 'sharded'], auth: 'disabled' },
+        requires: { topology: ['replicaset', 'sharded'] },
         // Skipping session leak tests b/c these are explicit sessions
-        sessions: { skipLeakTests: true },
-        skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+        sessions: { skipLeakTests: true }
       },
 
       test: function () {

diff --git a/test/integration/change-streams/change_stream.test.js b/test/integration/change-streams/change_stream.test.js
@@ -11,6 +11,7 @@ const { Long, ReadPreference, MongoNetworkError } = require('../../../src');
 
 const crypto = require('crypto');
 const { isHello } = require('../../../src/utils');
+const { skipBrokenAuthTestBeforeEachHook } = require('../../tools/runner/hooks/configuration');
 
 function withChangeStream(dbName, collectionName, callback) {
   if (arguments.length === 1) {
@@ -1253,6 +1254,12 @@ describe('Change Streams', function () {
       return coll.insertOne({ c: 3 });
     }
 
+    beforeEach(
+      skipBrokenAuthTestBeforeEachHook({
+        skippedTests: ['when invoked using eventEmitter API']
+      })
+    );
+
     beforeEach(function () {
       client = this.configuration.newClient();
       return client.connect().then(_client => {
@@ -1334,8 +1341,7 @@ describe('Change Streams', function () {
 
     it('when invoked using eventEmitter API', {
       metadata: {
-        requires: { topology: 'replicaset', mongodb: '>=3.6', auth: 'disabled' },
-        skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+        requires: { topology: 'replicaset', mongodb: '>=3.6', auth: 'disabled' }
       },
       test: function (done) {
         let closed = false;

diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.js b/test/integration/connection-monitoring-and-pooling/connection.test.js
@@ -8,8 +8,18 @@ const { setupDatabase, withClient, assert: test } = require('../shared');
 const { ns, HostAddress } = require('../../../src/utils');
 const { LEGACY_HELLO_COMMAND } = require('../../../src/constants');
 const { Topology } = require('../../../src/sdam/topology');
+const { skipBrokenAuthTestBeforeEachHook } = require('../../tools/runner/hooks/configuration');
 
 describe('Connection', function () {
+  beforeEach(
+    skipBrokenAuthTestBeforeEachHook({
+      skippedTests: [
+        'should support calling back multiple times on exhaust commands',
+        'should correctly connect to server using domain socket'
+      ]
+    })
+  );
+
   before(function () {
     return setupDatabase(this.configuration);
   });
@@ -89,8 +99,7 @@ describe('Connection', function () {
 
     it('should support calling back multiple times on exhaust commands', {
       metadata: {
-        requires: { apiVersion: false, mongodb: '>=4.2.0', topology: ['single'], auth: 'disabled' },
-        skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+        requires: { apiVersion: false, mongodb: '>=4.2.0', topology: ['single'] }
       },
       test: function (done) {
         const namespace = ns(`${this.configuration.db}.$cmd`);
@@ -200,8 +209,7 @@ describe('Connection', function () {
 
     it('should correctly connect to server using domain socket', {
       metadata: {
-        requires: { topology: 'single', os: '!win32', auth: 'disabled' },
-        skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+        requires: { topology: 'single', os: '!win32' }
       },
 
       test: function (done) {

diff --git a/test/integration/connections-survive-step-down/connections_survive_step_down.prose.test.js b/test/integration/connections-survive-step-down/connections_survive_step_down.prose.test.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const { expect } = require('chai');
+const { skipBrokenAuthTestBeforeEachHook } = require('../../tools/runner/hooks/configuration');
 
 function ignoreNsNotFound(err) {
   if (!err.message.match(/ns not found/)) {
@@ -33,6 +34,18 @@ maybeDescribe('Connections survive primary step down - prose', function () {
   let db;
   let collection;
 
+  beforeEach(
+    skipBrokenAuthTestBeforeEachHook({
+      skippedTests: [
+        'getMore iteration',
+        'Not Primary - Keep Connection Pool',
+        'Not Primary - Reset Connection Pool',
+        'Shutdown in progress - Reset Connection Pool',
+        'Interrupted at shutdown - Reset Connection Pool'
+      ]
+    })
+  );
+
   beforeEach(function () {
     const clientOptions = {
       maxPoolSize: 1,
@@ -67,14 +80,13 @@ maybeDescribe('Connections survive primary step down - prose', function () {
   afterEach(function () {
     return Promise.all(deferred.map(d => d())).then(() => {
       deferred = [];
-      return Promise.all([client.close(), checkClient.close()]);
+      return Promise.all([client, checkClient].filter(x => !!x).map(client => client.close()));
     });
   });
 
   it('getMore iteration', {
     metadata: {
-      requires: { mongodb: '>=4.2.0', topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { mongodb: '>=4.2.0', topology: 'replicaset' }
     },
 
     test: function () {
@@ -142,8 +154,7 @@ maybeDescribe('Connections survive primary step down - prose', function () {
 
   it('Not Primary - Keep Connection Pool', {
     metadata: {
-      requires: { mongodb: '>=4.2.0', topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { mongodb: '>=4.2.0', topology: 'replicaset' }
     },
     test: function () {
       return runStepownScenario(10107, expectPoolWasNotCleared);
@@ -152,8 +163,7 @@ maybeDescribe('Connections survive primary step down - prose', function () {
 
   it('Not Primary - Reset Connection Pool', {
     metadata: {
-      requires: { mongodb: '4.0.x', topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { mongodb: '4.0.x', topology: 'replicaset' }
     },
     test: function () {
       return runStepownScenario(10107, expectPoolWasCleared);
@@ -162,8 +172,7 @@ maybeDescribe('Connections survive primary step down - prose', function () {
 
   it('Shutdown in progress - Reset Connection Pool', {
     metadata: {
-      requires: { mongodb: '>=4.0.0', topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { mongodb: '>=4.0.0', topology: 'replicaset' }
     },
     test: function () {
       return runStepownScenario(91, expectPoolWasCleared);
@@ -172,8 +181,7 @@ maybeDescribe('Connections survive primary step down - prose', function () {
 
   it('Interrupted at shutdown - Reset Connection Pool', {
     metadata: {
-      requires: { mongodb: '>=4.0.0', topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { mongodb: '>=4.0.0', topology: 'replicaset' }
     },
     test: function () {
       return runStepownScenario(11600, expectPoolWasCleared);

diff --git a/test/integration/node-specific/operation_example.test.js b/test/integration/node-specific/operation_example.test.js
@@ -5,6 +5,7 @@ const { Topology } = require('../../../src/sdam/topology');
 const { Code, ObjectId, ReturnDocument } = require('../../../src');
 
 const chai = require('chai');
+const { skipBrokenAuthTestBeforeEachHook } = require('../../tools/runner/hooks/configuration');
 
 const expect = chai.expect;
 chai.use(require('chai-subset'));
@@ -14,6 +15,15 @@ describe('Operation Examples', function () {
     return setupDatabase(this.configuration, ['integration_tests_2']);
   });
 
+  beforeEach(
+    skipBrokenAuthTestBeforeEachHook({
+      skippedTests: [
+        'Should correctly connect to a replicaset',
+        'Should connect to mongos proxies using connectiong string'
+      ]
+    })
+  );
+
   /**************************************************************************
    *
    * COLLECTION TESTS
@@ -5488,8 +5498,7 @@ describe('Operation Examples', function () {
    */
   it('Should correctly connect to a replicaset', {
     metadata: {
-      requires: { topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { topology: 'replicaset' }
     },
 
     test: function (done) {
@@ -5544,8 +5553,7 @@ describe('Operation Examples', function () {
    */
   it('Should connect to mongos proxies using connectiong string', {
     metadata: {
-      requires: { topology: 'sharded', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { topology: 'sharded' }
     },
 
     test: function (done) {

diff --git a/test/integration/node-specific/operation_promises_example.test.js b/test/integration/node-specific/operation_promises_example.test.js
@@ -6,12 +6,23 @@ const { enumToString } = require('../../../src/utils');
 const { ProfilingLevel } = require('../../../src/operations/set_profiling_level');
 const { Code, ReturnDocument } = require('../../../src');
 const { expect } = require('chai');
+const { skipBrokenAuthTestBeforeEachHook } = require('../../tools/runner/hooks/configuration');
 
 describe('Operation (Promises)', function () {
   before(function () {
     return setupDatabase(this.configuration, ['integration_tests_2', 'hr', 'reporting']);
   });
 
+  beforeEach(
+    skipBrokenAuthTestBeforeEachHook({
+      skippedTests: [
+        'Should correctly connect to a replicaset',
+        'Should connect to mongos proxies using connectiong string With Promises',
+        'Should correctly connect to a replicaset With Promises'
+      ]
+    })
+  );
+
   /**************************************************************************
    *
    * COLLECTION TESTS
@@ -4016,8 +4027,7 @@ describe('Operation (Promises)', function () {
    */
   it('Should correctly connect to a replicaset With Promises', {
     metadata: {
-      requires: { topology: 'replicaset', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { topology: 'replicaset' }
     },
 
     test: function () {
@@ -4064,8 +4074,7 @@ describe('Operation (Promises)', function () {
    */
   it('Should connect to mongos proxies using connectiong string With Promises', {
     metadata: {
-      requires: { topology: 'sharded', auth: 'disabled' },
-      skipReason: 'TODO: NODE-3891 - fix tests broken when AUTH enabled'
+      requires: { topology: 'sharded' }
     },
 
     test: function () {

diff --git a/test/tools/reporter/mongodb_reporter.js b/test/tools/reporter/mongodb_reporter.js
@@ -233,11 +233,6 @@ class MongoDBMochaReporter extends mocha.reporters.Spec {
    */
   pending(test) {
     if (REPORT_TO_STDIO) console.log(chalk.cyan(`↬ ${test.fullTitle()}`));
-    if (test.metadata && test.metadata.skipReason && typeof test.metadata.skipReason === 'string') {
-      console.log(
-        chalk.cyan(`${'  '.repeat(test.titlePath().length + 1)}↬ ${test.metadata.skipReason}`)
-      );
-    }
     if (typeof test.skipReason === 'string') {
       console.log(chalk.cyan(`${'  '.repeat(test.titlePath().length + 1)}↬ ${test.skipReason}`));
     }

diff --git a/test/tools/runner/hooks/configuration.js b/test/tools/runner/hooks/configuration.js
@@ -88,6 +88,16 @@ const testSkipBeforeEachHook = async function () {
   }
 };
 
+// TODO: NODE-3891 - fix tests that are broken with auth enabled and remove this hook
+const skipBrokenAuthTestBeforeEachHook = function ({ skippedTests } = { skippedTests: [] }) {
+  return function () {
+    if (process.env.AUTH === 'auth' && skippedTests.includes(this.currentTest.title)) {
+      this.currentTest.skipReason = 'TODO: NODE-3891 - fix tests broken when AUTH enabled';
+      this.skip();
+    }
+  };
+};
+
 const testConfigBeforeHook = async function () {
   const client = new MongoClient(loadBalanced ? SINGLE_MONGOS_LB_URI : MONGODB_URI, {
     ...getEnvironmentalOptions()
@@ -152,5 +162,6 @@ module.exports = {
     beforeAll: [beforeAllPluginImports, testConfigBeforeHook],
     beforeEach: [testSkipBeforeEachHook],
     afterAll: [cleanUpMocksAfterHook]
-  }
+  },
+  skipBrokenAuthTestBeforeEachHook
 };