TOOLS-4148 Annotate JS tests with SKIP, EXTEND, or NEW after auditing to figure out which ones need to be converted to Go by autarch · Pull Request #922 · mongodb/mongo-tools

autarch · 2026-03-19T17:09:05Z

Adds a // MIGRATION: comment to the first line of every JS test file under test/qa-tests/ and test/legacy42/ indicating its migration status:

SKIP (27 files): genuinely covered by existing Go tests, broken pipe tests (OS-signal-level, not testable in Go), or already disabled
NEW (111 files): no Go test coverage exists for this scenario
EXTEND (41 files): partial Go coverage exists but specific sub-scenarios are missing

Each annotation includes the justification and the target Go test file.

autarch · 2026-03-19T17:09:20Z

TOOLS-4148 Annotate JS tests with SKIP, EXTEND, or NEW after auditing to figure out which ones need to be converted to Go #922 👈 (View in Graphite)
TOOLS-4148 Add a new sharded test type for sharded cluster integration tests #916 : 1 other dependent PR (#956 )
Fix linting ignore rule for bson types like bson.D #926
Add an AGENTS.md file for this repo #924
master

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

FGasper

I think we should preserve the SIGPIPE tests.
I’ll defer to you on whether existing Go tests do indeed obviate specific JS tests, but I’m suspicious.

FGasper · 2026-03-24T13:13:19Z

test/qa-tests/jstests/export/export_broken_pipe.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — broken pipe is OS-signal-level, not testable in Go


I disagree … ?

FGasper · 2026-03-24T13:14:15Z

test/qa-tests/jstests/dump/version_test.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — standard CLI --version flag, not mongo-specific


huh? The args given to mongodump here seem more interesting than this.

Yeah, the AI got this wrong. I already had it revisit all its SKIP markings earlier, and I thought I caught all the bogus ones, but apparently not. I will push an update to the plan that includes this as one to be converted.

FGasper · 2026-03-24T13:16:27Z

test/qa-tests/jstests/dump/dump_broken_pipe.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — broken pipe is OS-signal-level, not testable in Go


can def be tested in Go

FGasper · 2026-03-24T13:16:44Z

test/qa-tests/jstests/bson/bsondump_broken_pipe.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — broken pipe is OS-signal-level, not testable in Go


testable in Go

What exactly are we testing here? I did notice that we suppress SIGPIPE in the signal handling under common. The comment there says something about the tools checking the results of their write calls. This dates back to https://jira.mongodb.org/browse/TOOLS-1366, which says that sometimes network write errors would manifest as a SIGPIPE.

I've never heard of this, so I asked both Glean and Gemini. Both seemed to confirm that with modern Go versions, the Go runtime suppresses this behavior except when writing to stdout/stderr, so I don't think we can get a SIGPIPE error because of a network issue in the tools code.

Apparently, the Go runtime will convert the SIGPIPE from the kernel into an error that it returns from the network read or write call. However, it still does allow a SIGPIPE through when writing to stdout/stderr.

So given all this, I think we should revert the change to have the tools ignore the SIGPIPE signal. This doesn't achieve anything. And it never made sense for things like bsondump which don't even talk to the network.

Here's what it looks like if I run bsondump with the code to ignore SIGPIPE commented out:

go run ./bsondump/main/ --type=json ./test/qa-tests/jstests/bson/testdata/all_types.bson | dd count=0 bs=1 of=/dev/null 0+0 records in 0+0 records out 0 bytes copied, 3.1766e-05 s, 0.0 kB/s signal: broken pipe

And the current behavior, with the signal ignored:

go run ./bsondump/main/ --type=json ./test/qa-tests/jstests/bson/testdata/all_types.bson | dd count=0 bs=1 of=/dev/null 0+0 records in 0+0 records out 0 bytes copied, 2.3409e-05 s, 0.0 kB/s 2026-03-24T16:23:48.254-0500 0 objects found 2026-03-24T16:23:48.254-0500 write /dev/stdout: broken pipe exit status 1

The first seems more correct to me.

What do you think?

Given SIGPIPE’s history (a kludge for when folks “couldn’t be bothered” to check for failure), I actually favor ignoring it as long as we have linting in place that enforces error checking.

We do have errcheck enabled in this project's golangci-lint config.

I still don't see the point of leaving this as-is. But that's fine. I'll just add a single test in common for this behavior, since that's where the tools get this behavior.

Alright, see #956

FGasper · 2026-03-24T13:19:08Z

test/qa-tests/jstests/files/mongofiles_version.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — standard --version flag, not mongo-specific


This seems imprudently sensitive to implementation. It’s not a slow test or anything, so ISTM we might as well preserve this test.

Yeah, the AI hallucinated this too. That said, I really question some of these tests. It runs mongofiles --version with three different cluster types to test that it prints its version?! And why do we only test this for mongfiles but not any other tools.

I'll tell the AI to add a test to the code in common that generates this output.

FGasper · 2026-03-24T13:20:34Z

test/qa-tests/jstests/restore/different_collection.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — rename collection on restore covered by mongorestore_test.go


I’m a bit suspicious of these SKIPs. Just because something else tests renames doesn’t mean that that existing test flexes exactly the same code paths & does the same assertions.

Yeah, some of this isn't tested by the existing code. I'll update the plan.

FGasper · 2026-03-24T13:23:01Z

test/qa-tests/jstests/stat/stat_broken_pipe.js

@@ -1,3 +1,4 @@
+// MIGRATION: SKIP — broken pipe is OS-signal-level, not testable in Go


autarch · 2026-03-24T21:38:26Z

I updated the plan for the things you pointed out. For the pipe thing, I have a long comment. This stuff is a bit weird.

autarch · 2026-03-24T21:38:54Z

I'd also note that for many of the existing broken pipe tests, we've actually commented out the part that tests the failure case because apparently the JS test was flaky. So it really isn't testing anything!

Adds a // MIGRATION: comment to the first line of every JS test file under test/qa-tests/ and test/legacy42/ indicating its migration status: - SKIP (27 files): genuinely covered by existing Go tests, broken pipe tests (OS-signal-level, not testable in Go), or already disabled - NEW (111 files): no Go test coverage exists for this scenario - EXTEND (41 files): partial Go coverage exists but specific sub-scenarios are missing Each annotation includes the justification and the target Go test file. Co-Authored-By: Claude Code <noreply@anthropic.com>

autarch · 2026-03-25T16:54:26Z

I'm going to close this PR. After some discussion, it seems like there's broad agreement that we don't want this sort of AI-generated work planning to be committed, since it just adds noise. But I think having you review the AI's reasoning was quite helpful, @FGasper!

This was referenced Mar 19, 2026

TOOLS-4148 Add a new sharded test type for sharded cluster integration tests #916

Merged

TOOLS-4148 Rewrite all the bsondump tests in Go and delete the JS tests for this #923

Open

autarch force-pushed the 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests branch from f8d80ef to b8c834c Compare March 20, 2026 16:53

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 2d4c1a3 to 8602a4d Compare March 20, 2026 16:53

autarch changed the base branch from 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests to graphite-base/922 March 20, 2026 18:14

autarch mentioned this pull request Mar 20, 2026

Add an AGENTS.md file for this repo #924

Merged

autarch force-pushed the graphite-base/922 branch from b8c834c to a3170ff Compare March 20, 2026 18:31

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 8602a4d to 951bc97 Compare March 20, 2026 18:31

autarch changed the base branch from graphite-base/922 to 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests March 20, 2026 18:32

autarch force-pushed the 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests branch from a3170ff to e787652 Compare March 20, 2026 20:34

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 951bc97 to 7a3fa74 Compare March 20, 2026 20:34

autarch force-pushed the 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests branch from e787652 to 7a9eeeb Compare March 20, 2026 20:35

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 7a3fa74 to be4d1de Compare March 20, 2026 20:35

autarch mentioned this pull request Mar 20, 2026

TOOLS-4148 Convert test/qa-tests/jstests/export/basic_data.js to Go #925

Open

autarch force-pushed the 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests branch from 7a9eeeb to fbdcd63 Compare March 20, 2026 21:26

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from be4d1de to 13b1165 Compare March 20, 2026 21:26

This was referenced Mar 20, 2026

Fix linting ignore rule for bson types like bson.D #926

Merged

TOOLS-4148 Convert test/qa-tests/jstests/export/data_types.js to Go #927

Open

autarch marked this pull request as ready for review March 20, 2026 21:27

autarch requested a review from a team as a code owner March 20, 2026 21:27

autarch requested review from FGasper and tdq45gj and removed request for a team and tdq45gj March 20, 2026 21:27

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 13b1165 to aae3a48 Compare March 23, 2026 15:07

autarch force-pushed the 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests branch 2 times, most recently from 00b26a2 to 493873b Compare March 23, 2026 15:30

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from aae3a48 to 66aa6b5 Compare March 23, 2026 15:30

autarch mentioned this pull request Mar 23, 2026

TOOLS-4148 Convert test/qa-tests/jstests/export/export_views.js to Go #928

Open

FGasper requested changes Mar 24, 2026

View reviewed changes

autarch changed the title ~~Annotate JS tests with SKIP, EXTEND, or NEW after auditing to figure out which ones need to be converted to Go~~ TOOLS-4148 Annotate JS tests with SKIP, EXTEND, or NEW after auditing to figure out which ones need to be converted to Go Mar 24, 2026

autarch mentioned this pull request Mar 24, 2026

TOOLS-4148 Convert test/qa-tests/jstests/import/import_document_validation.js to Go #953

Open

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 66aa6b5 to 8871966 Compare March 24, 2026 21:39

autarch force-pushed the 03-18-migration_annotate_js_tests_with_verified_go_coverage_triage branch from 8871966 to 17ad8a1 Compare March 25, 2026 16:50

autarch force-pushed the 03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests branch from 493873b to 21e4728 Compare March 25, 2026 16:50

autarch mentioned this pull request Mar 25, 2026

TOOLS-4148 Add tests for SIGPIPE handling, remove equivalent JS tests, and fix handling for other signals #956

Open

autarch closed this Mar 25, 2026

This was referenced Mar 26, 2026

TOOLS-4148 Convert test/qa-tests/jstests/import/mongoimport mode.js to Go #960

Open

TOOLS-4148 Convert mode_upsert_id_subdoc.js to Go #964

Draft

Add a Claude skill for converting JS tests to Go #965

Draft

		@@ -1,3 +1,4 @@
		// MIGRATION: SKIP — broken pipe is OS-signal-level, not testable in Go

		@@ -1,3 +1,4 @@
		// MIGRATION: SKIP — standard CLI --version flag, not mongo-specific

		@@ -1,3 +1,4 @@
		// MIGRATION: SKIP — standard --version flag, not mongo-specific

		@@ -1,3 +1,4 @@
		// MIGRATION: SKIP — rename collection on restore covered by mongorestore_test.go

Conversation

autarch commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

autarch commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FGasper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

autarch commented Mar 24, 2026

Uh oh!

autarch commented Mar 24, 2026

Uh oh!

autarch commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

autarch commented Mar 19, 2026 •

edited

Loading

autarch commented Mar 19, 2026 •

edited

Loading