GODRIVER-2002 Remove 'slaveok' from source code and most spec tests #713
Conversation
| @@ -24,6 +24,7 @@ var ( | |||
| "collection-management", | |||
| "command-monitoring/unified", | |||
| "sessions/unified", | |||
| "retryable-writes/unified", | |||
There was a problem hiding this comment.
I synced the retryable-writes spec tests. This is unrelated to oppressive language removal, but it's always good to update tests as long as they pass. Some retryable writes tests had been ported over to the unified test format, so I added them to the test runner.
| @@ -437,6 +437,19 @@ func executeTestRunnerOperation(mt *mtest.T, testCase *testCase, op *operation, | |||
| fp, err := op.Arguments.LookupErr("failPoint") | |||
| assert.Nil(mt, err, "failPoint not found in arguments") | |||
| mt.SetFailPointFromDocument(fp.Document()) | |||
| case "assertSessionTransactionState": | |||
There was a problem hiding this comment.
We were missing this test runner operation. It's needed in some newly updated transactions/legacy tests.
There was a problem hiding this comment.
Changes look good.
I checked with a grep command modified from this comment.
ggrep -PriI 'slave' --exclude-dir=vendor --exclude-dir=.git .
That shows "slave" in the title or contents of SDAM integration tests. Comparing with the specifications repo I think those have been updated. Should those be updated in this PR?
| assert.Nil(mt, err, "state not found in arguments") | ||
| expectedState, ok := stateVal.StringValueOK() | ||
| assert.True(mt, ok, "state argument is not a string") | ||
| actualState := clientSession.TransactionState.String() |
There was a problem hiding this comment.
Nit: should this also assert clientSession is not nil?
There was a problem hiding this comment.
That's a good idea. I think the only time clientSession would be nil is if there was a malformed test with an assertSessionTransactionState and no corresponding session argument, but it's still worth having a check. There are numerous assertions on the fields of clientSession in this function without preceding NotNil assertions on clientSession, so I filed GODRIVER-2117.
GODRIVER-2002
Updates a number of spec tests that contain variations of
slaveokto use variations ofsecondaryok. RemovesSlaveOkfrom source code and replaces it withSecondaryOk.The Go driver did not parse a
slaveOKconnection string option, so there was no work there.