Conversation
This was referenced Mar 25, 2026
Collaborator
Author
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
505e342 to
0d86363
Compare
autarch
changed the title
common codecommon code and remove JS tests for broken pipe handling
autarch
changed the title
common code and remove JS tests for broken pipe handlingcommon, remove JS tests for broken pipe handling, and fix handling for other signals
autarch
changed the base branch from
03-18-migration_annotate_js_tests_with_verified_go_coverage_triage
to
graphite-base/956
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
0d86363 to
92b91bd
Compare
autarch
force-pushed
the
graphite-base/956
branch
from
17ad8a1 to
1085693
Compare
autarch
changed the base branch from
graphite-base/956
to
03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
2 times, most recently
from
646a13b to
a8eb9e7
Compare
autarch
force-pushed
the
03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests
branch
from
1085693 to
766992b
Compare
autarch
force-pushed
the
03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests
branch
from
766992b to
aede66b
Compare
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
a8eb9e7 to
0fa5fbe
Compare
autarch
changed the base branch from
03-13-add_a_new_sharded_test_type_for_sharded_cluster_integration_tests
to
graphite-base/956
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
0fa5fbe to
de973e6
Compare
autarch
force-pushed
the
graphite-base/956
branch
from
aede66b to
48c54ef
Compare
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
de973e6 to
d93b762
Compare
FGasper
requested changes
Mar 26, 2026
Collaborator
There was a problem hiding this comment.
This replaces end-to-end tests of the actual applications with a unit test of a specific function. It’s useful but doesn’t retain the original test’s scope.
How feasible is it to build the application, pipe its stdout to a process that ends right away, and verify that the application exits nonzero but doesn’t die from SIGPIPE? (Probably also keep the check for the string broken pipe.)
autarch
changed the title
common, remove JS tests for broken pipe handling, and fix handling for other signals
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
d93b762 to
dc868fb
Compare
Collaborator
Author
|
I added end to end tests for bsondump, mongodump, and mongoexport. |
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
dc868fb to
a7a8c12
Compare
This test makes sure that we ignore SIGPIPE. Instead, writes to a broken pipe will cause the write call to return an error. The tools should always check errors on write calls, so these will be surfaced as normal "failure to write to an fd" errors, as opposed to the signal aborting the process.
autarch
force-pushed
the
03-25-add_a_new_test_in_common_for_our_handling_of_sigpipe
branch
from
a7a8c12 to
8887a8d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This test makes sure that we ignore SIGPIPE. Instead, writes to a broken pipe will cause the write call to return an error. The tools should always check errors on write calls, so these will be surfaced as normal "failure to write to an fd" errors, as opposed to the signal aborting the process.
This also fixes a bug in our handling of other channels that the AI found. In Go, we should always use a buffered channel for signal handling, or we can miss the signal entirely. See https://pkg.go.dev/os/signal for more on this.