test: update SDAM spec tests to support multiple phases

Author: mbroadst

test/functional/spec-runner/matcher.js

@@ -103,6 +103,12 @@ function generateMatchAndDiffSpecialCase(key, expectedObj, actualObj, metadata)
       expected: match ? actual : SYMBOL_DOES_EXIST,
       actual
     };
+  } else if (expectedIs42) {
+    return {
+      match: true,
+      expected: SYMBOL_IGNORE,
+      actual: SYMBOL_IGNORE
+    };
   } else {
     // default
     return generateMatchAndDiff(expected, actual, metadata);

test/spec/server-discovery-and-monitoring/README.rst

@@ -17,14 +17,17 @@ Format
 
 Each YAML file has the following keys:
 
-- description: Some text.
+- description: A textual description of the test.
 - uri: A connection string.
 - phases: An array of "phase" objects.
+  A phase of the test optionally sends inputs to the client,
+  then tests the client's resulting TopologyDescription.
 
-A "phase" of the test sends inputs to the client, then tests the client's
-resulting TopologyDescription. Each phase object has two keys:
+Each phase object has two keys:
 
-- responses: An array of "response" objects.
+- responses: (optional) An array of "response" objects. If not provided,
+  the test runner should construct the client and perform assertions specified
+  in the outcome object without processing any responses.
 - outcome: An "outcome" object representing the TopologyDescription.
 
 A response is a pair of values:
@@ -37,8 +40,9 @@ A response is a pair of values:
   The empty response `{}` indicates a network error
   when attempting to call "ismaster".
 
-An "outcome" represents the correct TopologyDescription that results from
-processing the responses in the phases so far. It has the following keys:
+In non-monitoring tests, an "outcome" represents the correct
+TopologyDescription that results from processing the responses in the phases
+so far. It has the following keys:
 
 - topologyType: A string like "ReplicaSetNoPrimary".
 - setName: A string with the expected replica set name, or null.
@@ -60,6 +64,16 @@ current TopologyDescription. It has the following keys:
 - minWireVersion: absent or an integer.
 - maxWireVersion: absent or an integer.
 
+In monitoring tests, an "outcome" contains a list of SDAM events that should
+have been published by the client as a result of processing ismaster responses
+in the current phase. Any SDAM events published by the client during its
+construction (that is, prior to processing any of the responses) should be
+combined with the events published during processing of ismaster responses
+of the first phase of the test. A test MAY explicitly verify events published
+during client construction by providing an empty responses array for the
+first phase.
+
+
 Use as unittests
 ----------------
 
@@ -71,7 +85,7 @@ without any network I/O, by parsing ismaster responses from the test file
 and passing them into the driver code. Parts of the client and monitoring
 code may need to be mocked or subclassed to achieve this. `A reference
 implementation for PyMongo 3.x is available here
-<https://github.com/mongodb/mongo-python-driver/blob/3.0-dev/test/test_discovery_and_monitoring.py>`_.
+<https://github.com/mongodb/mongo-python-driver/blob/26d25cd74effc1e7a8d52224eac6c9a95769b371/test/test_discovery_and_monitoring.py>`_.
 
 Initialization
 ~~~~~~~~~~~~~~
@@ -93,19 +107,29 @@ Set the client's initial TopologyType to ReplicaSetNoPrimary.
 (For most clients, parsing a connection string with a "replicaSet" option
 automatically sets the TopologyType to ReplicaSetNoPrimary.)
 
+Set up a listener to collect SDAM events published by the client, including
+events published during client construction.
+
 Test Phases
 ~~~~~~~~~~~
 
 For each phase in the file, parse the "responses" array.
 Pass in the responses in order to the driver code.
 If a response is the empty object `{}`, simulate a network error.
 
-Once all responses are processed, assert that the phase's "outcome" object
+For non-monitoring tests,
+once all responses are processed, assert that the phase's "outcome" object
 is equivalent to the driver's current TopologyDescription.
 
+For monitoring tests, once all responses are processed, assert that the
+events collected so far by the SDAM event listener are equivalent to the
+events specified in the phase.
+
 Some fields such as "logicalSessionTimeoutMinutes" or "compatible" were added
 later and haven't been added to all test files. If these fields are present,
 test that they are equivalent to the fields of the driver's current
 TopologyDescription.
 
+For monitoring tests, clear the list of events collected so far.
+
 Continue until all phases have been executed.

test/spec/server-discovery-and-monitoring/monitoring/replica_set_with_removal.json

@@ -3,30 +3,7 @@
   "uri": "mongodb://a,b/",
   "phases": [
     {
-      "responses": [
-        [
-          "a:27017",
-          {
-            "ok": 1,
-            "ismaster": true,
-            "setName": "rs",
-            "setVersion": 1,
-            "primary": "a:27017",
-            "hosts": [
-              "a:27017"
-            ],
-            "minWireVersion": 0,
-            "maxWireVersion": 4
-          }
-        ],
-        [
-          "b:27017",
-          {
-            "ok": 1,
-            "ismaster": true
-          }
-        ]
-      ],
+      "responses": [],
       "outcome": {
         "events": [
           {
@@ -73,7 +50,37 @@
               "topologyId": "42",
               "address": "b:27017"
             }
-          },
+          }
+        ]
+      }
+    },
+    {
+      "responses": [
+        [
+          "a:27017",
+          {
+            "ok": 1,
+            "ismaster": true,
+            "setName": "rs",
+            "setVersion": 1,
+            "primary": "a:27017",
+            "hosts": [
+              "a:27017"
+            ],
+            "minWireVersion": 0,
+            "maxWireVersion": 4
+          }
+        ],
+        [
+          "b:27017",
+          {
+            "ok": 1,
+            "ismaster": true
+          }
+        ]
+      ],
+      "outcome": {
+        "events": [
           {
             "server_description_changed_event": {
               "topologyId": "42",

test/spec/server-discovery-and-monitoring/monitoring/replica_set_with_removal.yml

@@ -2,22 +2,7 @@ description: "Monitoring a replica set with non member"
 uri: "mongodb://a,b/"
 phases:
   -
-    responses:
-      -
-        - "a:27017"
-        - {
-            ok: 1,
-            ismaster: true,
-            setName: "rs",
-            setVersion: 1.0,
-            primary: "a:27017",
-            hosts: [ "a:27017" ],
-            minWireVersion: 0,
-            maxWireVersion: 4
-          }
-      -
-        - "b:27017"
-        - { ok: 1, ismaster: true }
+    responses: []
     outcome:
       events:
         -
@@ -52,6 +37,25 @@ phases:
           server_opening_event:
             topologyId: "42"
             address: "b:27017"
+  -
+    responses:
+      -
+        - "a:27017"
+        - {
+            ok: 1,
+            ismaster: true,
+            setName: "rs",
+            setVersion: 1.0,
+            primary: "a:27017",
+            hosts: [ "a:27017" ],
+            minWireVersion: 0,
+            maxWireVersion: 4
+          }
+      -
+        - "b:27017"
+        - { ok: 1, ismaster: true }
+    outcome:
+      events:
         -
           server_description_changed_event:
             topologyId: "42"

test/spec/server-discovery-and-monitoring/monitoring/standalone_debounce_description_updates.json renamed to test/spec/server-discovery-and-monitoring/monitoring/standalone_suppress_equal_description_changes.json

@@ -1,5 +1,5 @@
 {
-  "description": "Monitoring a standalone connection - multiple equal description updates are debounced",
+  "description": "Monitoring a standalone connection - suppress update events for equal server descriptions",
   "uri": "mongodb://a:27017",
   "phases": [
     {

test/spec/server-discovery-and-monitoring/monitoring/standalone_debounce_description_updates.yml renamed to test/spec/server-discovery-and-monitoring/monitoring/standalone_suppress_equal_description_changes.yml

@@ -1,4 +1,4 @@
-description: "Monitoring a standalone connection - multiple equal description updates are debounced"
+description: "Monitoring a standalone connection - suppress update events for equal server descriptions"
 uri: "mongodb://a:27017"
 phases:
   -

test/spec/server-discovery-and-monitoring/rs/incompatible_ghost.json

@@ -23,11 +23,6 @@
           {
             "ok": 1,
             "isreplicaset": true,
-            "setName": "rs",
-            "hosts": [
-              "a:27017",
-              "b:27017"
-            ],
             "minWireVersion": 0,
             "maxWireVersion": 1
           }
@@ -41,7 +36,7 @@
           },
           "b:27017": {
             "type": "RSGhost",
-            "setName": "rs"
+            "setName": null
           }
         },
         "topologyType": "ReplicaSetWithPrimary",

test/spec/server-discovery-and-monitoring/rs/incompatible_ghost.yml

@@ -14,8 +14,6 @@ phases:
         - "b:27017"
         - ok: 1
           isreplicaset: true
-          setName: "rs"
-          hosts: ["a:27017", "b:27017"]
           minWireVersion: 0
           maxWireVersion: 1
     outcome:
@@ -25,7 +23,7 @@ phases:
           setName: "rs"
         "b:27017":
           type: "RSGhost"
-          setName: "rs"
+          setName:
       topologyType: "ReplicaSetWithPrimary"
       setName: "rs"
       logicalSessionTimeoutMinutes: ~

test/unit/core/sdam_spec.test.js

@@ -1,16 +1,18 @@
 'use strict';
 const fs = require('fs');
 const path = require('path');
-const chai = require('chai');
-const expect = chai.expect;
 const Topology = require('../../../lib/core/sdam/topology').Topology;
 const Server = require('../../../lib/core/sdam/server').Server;
 const ServerDescription = require('../../../lib/core/sdam/server_description').ServerDescription;
 const monitoring = require('../../../lib/core/sdam/monitoring');
 const parse = require('../../../lib/core/uri_parser');
-chai.use(require('chai-subset'));
 const sinon = require('sinon');
 
+const chai = require('chai');
+chai.use(require('chai-subset'));
+chai.use(require('../../functional/spec-runner/matcher').default);
+const expect = chai.expect;
+
 const specDir = path.resolve(__dirname, '../../spec/server-discovery-and-monitoring');
 function collectTests() {
   const testTypes = fs
@@ -115,36 +117,6 @@ function findOmittedFields(expected) {
   return result;
 }
 
-function replacePlaceholders(actual, expected) {
-  Object.keys(expected).forEach(key => {
-    if (expected[key] === 42 || expected[key] === '42') {
-      expect(actual).to.have.any.keys(key);
-      expect(actual[key]).to.exist;
-      actual[key] = expected[key];
-    }
-  });
-
-  return actual;
-}
-
-function convertES6Maps(actual) {
-  ['previousDescription', 'newDescription'].forEach(key => {
-    if (actual[key] && actual[key].servers) {
-      const servers = actual[key].servers;
-      if (servers instanceof Map) {
-        let obj = Object.create(null);
-        for (const serverEntry of servers.entries()) {
-          obj[serverEntry[0]] = serverEntry[1];
-        }
-
-        actual[key].servers = obj;
-      }
-    }
-  });
-
-  return actual;
-}
-
 function normalizeServerDescription(serverDescription) {
   if (serverDescription.type === 'PossiblePrimary') {
     // Some single-threaded drivers care a lot about ordering potential primary
@@ -156,6 +128,26 @@ function normalizeServerDescription(serverDescription) {
   return serverDescription;
 }
 
+function cloneMap(map) {
+  const result = Object.create(null);
+  for (let key of map.keys()) {
+    result[key] = JSON.parse(JSON.stringify(map.get(key)));
+  }
+
+  return result;
+}
+
+function cloneForCompare(event) {
+  const result = JSON.parse(JSON.stringify(event));
+  ['previousDescription', 'newDescription'].forEach(key => {
+    if (event[key] != null && event[key].servers != null) {
+      result[key].servers = cloneMap(event[key].servers);
+    }
+  });
+
+  return result;
+}
+
 function executeSDAMTest(testData, testDone) {
   parse(testData.uri, (err, parsedUri) => {
     if (err) return done(err);
@@ -164,7 +156,7 @@ function executeSDAMTest(testData, testDone) {
     const topology = new Topology(parsedUri.hosts, parsedUri.options);
 
     // listen for SDAM monitoring events
-    const events = [];
+    let events = [];
     [
       'serverOpening',
       'serverClosed',
@@ -234,8 +226,8 @@ function executeSDAMTest(testData, testDone) {
           expect(events).to.have.length(expectedEvents.length);
           for (let i = 0; i < events.length; ++i) {
             const expectedEvent = expectedEvents[i];
-            const actualEvent = convertES6Maps(replacePlaceholders(events[i], expectedEvent));
-            expect(actualEvent).to.containSubset(expectedEvent);
+            const actualEvent = cloneForCompare(events[i]);
+            expect(actualEvent).to.matchMongoSpec(expectedEvent);
           }
 
           return;
@@ -252,6 +244,9 @@ function executeSDAMTest(testData, testDone) {
 
       // remove error handler
       topology.removeListener('error', incompatabilityHandler);
+
+      // reset the captured events for each phase
+      events = [];
     });
 
     topology.close(done);