GODRIVER-1931 Sync new spec tests #646
Conversation
divjotarora
force-pushed
the
godriver1931-sync-new-tests
branch
2 times, most recently
from
5e3e69c to
d2e9e1d
Compare
| @@ -112,6 +112,9 @@ func VerifyConnStringOptions(t *testing.T, cs connstring.ConnString, options map | |||
| case "journal": | |||
| require.True(t, cs.JSet) | |||
| require.Equal(t, value, cs.J) | |||
| case "loadbalanced": | |||
There was a problem hiding this comment.
This is required by URI options tests.
| @@ -60,7 +60,9 @@ type readPref struct { | |||
| TagSets []map[string]string `json:"tag_sets"` | |||
| } | |||
|
|
|||
| func topologyKindFromString(s string) TopologyKind { | |||
| func topologyKindFromString(t *testing.T, s string) TopologyKind { | |||
There was a problem hiding this comment.
The change to accept a testing.T here and in serverKindFromString aren't strictly required, but it's good practice for us to fail on unrecognized topology/server kind strings rather than defaulting to Unknown as we were before, so I felt it was best to add it.
There was a problem hiding this comment.
Nice attention to detail!
| case "LoadBalancer": | ||
| return LoadBalancer | ||
| case "PossiblePrimary", "Unknown": | ||
| // Go does not have a PossiblePrimary server type and per the SDAM spec, this type is synonymous with Unknown. |
There was a problem hiding this comment.
See ServerType.
| @@ -36,13 +36,22 @@ type seedlistTest struct { | |||
| } | |||
|
|
|||
| func TestInitialDNSSeedlistDiscoverySpec(t *testing.T) { | |||
There was a problem hiding this comment.
The changes here are to account for the fact that the initial DNS seedlist discovery tests were split into replica-set and load-balanced sub-directories. Each sub-test corresponds to one sub-directory.
| @@ -122,6 +131,10 @@ func verifyConnstringOptions(mt *mtest.T, expected bson.Raw, cs connstring.ConnS | |||
| dc := opt.Boolean() | |||
| assert.True(mt, cs.DirectConnectionSet, "expected cs.DirectConnectionSet to be true, got false") | |||
| assert.Equal(mt, dc, cs.DirectConnection, "expected cs.DirectConnection to be %v, got %v", dc, cs.DirectConnection) | |||
| case "loadBalanced": | |||
There was a problem hiding this comment.
This was added because loadBalanced can be present in TXT records.
| assert.False(t, topo.pollingRequired, "expected SRV polling to not be required, but it is") | ||
| }) | ||
|
|
||
| t.Run("new records are not detected", func(t *testing.T) { |
There was a problem hiding this comment.
This test is required by mongodb/specifications#934.
divjotarora
force-pushed
the
godriver1931-sync-new-tests
branch
from
d2e9e1d to
6ccc7e6
Compare
| @@ -60,7 +60,9 @@ type readPref struct { | |||
| TagSets []map[string]string `json:"tag_sets"` | |||
| } | |||
|
|
|||
| func topologyKindFromString(s string) TopologyKind { | |||
| func topologyKindFromString(t *testing.T, s string) TopologyKind { | |||
There was a problem hiding this comment.
Nice attention to detail!
| LookupTXT: mockResolver.LookupTXT, | ||
| } | ||
|
|
||
| topo := createLBTopology(t, "mongodb+srv://test3.test.build.10gen.cc/?loadBalanced=true") |
There was a problem hiding this comment.
Nit: is ?loadBalanced=true redundant here since createLBTopology sets the topology's loadbalanced option?
There was a problem hiding this comment.
Good catch. Removed. I also realized that createLBTopology is not setting the appropriate server/connection options to enable LB mode at all levels. This probably isn't relevant for this specific test, but I refactored it a bit to set LoadBalanced on the parsed ConnString instead so those options are all handled by WithConnString.
| topo := createLBTopology(t, "mongodb+srv://test3.test.build.10gen.cc/?loadBalanced=true") | ||
| topo.dnsResolver = dnsResolver | ||
| topo.rescanSRVInterval = time.Millisecond * 5 | ||
| err := topo.Connect() |
This PR syncs the spec tests introduced in mongodb/specifications#934, mongodb/specifications#958, and mongodb/specifications#961.