Introduce global::with_locked_writer
#43
Merged
Conversation
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
schneems
force-pushed
the
schneems/global-test-capture
branch
3 times, most recently
from
1a65050 to
678f608
Compare
_GlobalWriter is deprecated for GlobalWriter
schneems
force-pushed
the
schneems/global-test-capture
branch
from
678f608 to
4eeb69b
Compare
New function `global::with_locked_writer` is introduced to allow consistently capturing write output. This function is designed for use in testing output or in other non-reentrant capture cases. This blocks all threads using this function but one from executing so that a deterministic and consistent output is captured. Previously tests could be written with a thread_local writer, however there's a subtle race condition in that approach if the output relies on "paragraph" style text. That race condition occurs because `global::set_writer` not only mutates the writer but also resets the trailing newlines count, which affects the output. This replacement for that behavior should be safer.
These are a doc tests, so they already run in their own processes. Now these tests don't need a tempfile, which is nice.
schneems
force-pushed
the
schneems/global-test-capture
branch
from
4eeb69b to
031aa37
Compare
colincasey
reviewed
Jun 9, 2025
colincasey
reviewed
Jun 9, 2025
colincasey
reviewed
Jun 9, 2025
colincasey
approved these changes
Jun 9, 2025
schneems
added a commit
that referenced
this pull request
Jul 1, 2025
An issue was mentioned in heroku/buildpacks-deb-packages#110 (comment). This code was added in https://github.com/heroku-buildpacks/bullet_stream/pull/43/files and uses a feature of Rust 1.86 https://www.reddit.com/r/rust/comments/1ip51qt/trait_upcasting_stabilized_in_186/ (rust-lang/rust#134367). This usage wasn't realized at the time. No lints or tooling caught it. That Any upcast is critical for the feature introduced in #43. The easiest path forward is to align the MSRV with the reality of the current code. Semver version bump guidance from this thread to make this a minor change: https://users.rust-lang.org/t/rust-version-requirement-change-as-semver-breaking-or-not/20980/2.
schneems
added a commit
that referenced
this pull request
Jul 1, 2025
An issue was mentioned in heroku/buildpacks-deb-packages#110 (comment). This code was added in https://github.com/heroku-buildpacks/bullet_stream/pull/43/files and uses a feature of Rust 1.86 https://www.reddit.com/r/rust/comments/1ip51qt/trait_upcasting_stabilized_in_186/ (rust-lang/rust#134367). This usage wasn't realized at the time. No lints or tooling caught it. That Any upcast is critical for the feature introduced in #43. The easiest path forward is to align the MSRV with the reality of the current code. Semver version bump guidance from this thread to make this a minor change: https://users.rust-lang.org/t/rust-version-requirement-change-as-semver-breaking-or-not/20980/2.
schneems
added a commit
that referenced
this pull request
Jul 1, 2025
* Align minimum rust version to 1.86 An issue was mentioned in heroku/buildpacks-deb-packages#110 (comment). This code was added in https://github.com/heroku-buildpacks/bullet_stream/pull/43/files and uses a feature of Rust 1.86 https://www.reddit.com/r/rust/comments/1ip51qt/trait_upcasting_stabilized_in_186/ (rust-lang/rust#134367). This usage wasn't realized at the time. No lints or tooling caught it. That Any upcast is critical for the feature introduced in #43. The easiest path forward is to align the MSRV with the reality of the current code. Semver version bump guidance from this thread to make this a minor change: https://users.rust-lang.org/t/rust-version-requirement-change-as-semver-breaking-or-not/20980/2. * Fix clippy lint ``` error: variables can be used directly in the `format!` string --> src/global.rs:678:9 | 678 | assert!(result.is_err(), "Expected panic to be caught {:?}", result); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args = note: `-D clippy::uninlined-format-args` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::uninlined_format_args)]` help: change this to | 678 - assert!(result.is_err(), "Expected panic to be caught {:?}", result); 678 + assert!(result.is_err(), "Expected panic to be caught {result:?}"); | error: could not compile `bullet_stream` (lib test) due to 1 previous error ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
New function
global::with_locked_writeris introduced to allow consistently capturing write output. This function is designed for use in testing output or in other non-reentrant capture cases. This blocks all threads using this function but one from executing so that a deterministic and consistent output is captured. Previously tests could be written with a thread_local writer, however there's a subtle race condition in that approach if the output relies on "paragraph" style text.The cause of that race condition is because
global::set_writernot only mutates the writer, it also resets the trailing newlines count, which affects the output. This replacement for that behavior should be safer.GUS-W-18738519