Skip to content

TOOLS-4278 Convert three smallest GoConvey test files to testify - #1061

Draft
autarch wants to merge 1 commit into
masterfrom
TOOLS-4278-convey-to-testify-conventions
Draft

TOOLS-4278 Convert three smallest GoConvey test files to testify#1061
autarch wants to merge 1 commit into
masterfrom
TOOLS-4278-convey-to-testify-conventions

Conversation

@autarch

@autarch autarch commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Establishes the conventions for the rest of the goconvey -> testify
migration:

  • One top-level Convey means no subtest; multiple become t.Run. A Convey
    inside a loop is N top-level Conveys, so each case gets its own t.Run.
  • Nested Conveys that differ only in data collapse into table-driven
    cases rather than nested subtests.
  • require by default, assert where continuing gives more useful output.
  • Direct t.Fatal/t.Errorf assertions become require/assert.
  • Assertion messages describe the expected behavior, not the failure.

Also removes the repo's only cmp.Diff call and its go-cmp import.

No behavior change.

autarch commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Establishes the conventions for the rest of the goconvey -> testify
migration:

- One top-level Convey means no subtest; multiple become t.Run. A Convey
  inside a loop is N top-level Conveys, so each case gets its own t.Run.
- Nested Conveys that differ only in data collapse into table-driven
  cases rather than nested subtests.
- require by default, assert where continuing gives more useful output.
- Direct t.Fatal/t.Errorf assertions become require/assert.
- Assertion messages describe the expected behavior, not the failure.

Also removes the repo's only cmp.Diff call and its go-cmp import.

No behavior change.
@autarch

autarch commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Running the tests for this stack

Two flags matter, and without them you will see failures that look like they belong to this migration but don't.

Pass -tags failpoints

common/failpoint/manager.go is //go:build failpoints; without the tag noop_manager.go compiles instead and DefaultManager.Get always returns false. TestMongoDumpTOOLS2498 and TestFailDuringResharding then fail with nothing to indicate why. This fooled our own baseline for most of the migration — we tracked both as pre-existing failures until we read the build tags.

Run integration packages one at a time, from a clean server

mongodump and mongorestore both use todb/fromdb on the same server, so a plain go test ./... produces a spurious TestMongoDumpBSON failure from the two racing. Use -p 1.

Several integration tests also don't drop their databases, so the suite is not idempotent — a second full run fails in places the first didn't. Clear them in between:

mongo --quiet --port 33333 --eval 'db.getSiblingDB("fromdb").dropDatabase(); db.getSiblingDB("todb").dropDatabase(); db.getSiblingDB("mongoimport_booleantype_test").dropDatabase()'

Putting it together:

TOOLS_TESTING_UNIT=true TOOLS_TESTING_INTEGRATION=true go test -tags failpoints -p 1 -count=1 ./...

Failures that are real and predate this stack

Two tests fail on master as well, and are preserved rather than fixed — this stack is a mechanical refactor, so weak or broken tests were carried over as-is:

  • mongorestore / TestReadDumpServerVersionFromArchive — expects db.Version{8,0,16}, gets {8,0,17}. Our local mongod is a git build reporting 8.0.16-353-gb19191c, and the archive round trip normalizes that pre-release suffix inconsistently. Presumably green in CI against a release build.
  • mongofiles / TestDefaultWriteConcern — compares a *wcwrapper.WriteConcern against writeconcern.Majority(), which returns a bare *writeconcern.WriteConcern. Different types, so the deep-equal can never pass regardless of the values.

Also worth knowing: mongorestore takes ~30s against a healthy server but we hit one local replica set where it took 650s+ and blew Go's 10-minute package timeout. That reproduces identically on master, so if the package starts taking minutes, suspect the server rather than the tests.

Everything else

Green: mongodump, mongoimport, mongoexport, mongorestore/ns, and the full unit suite across the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant