Skip to content

Conversation

@Imberflur
Copy link
Contributor

Fixes #59

This is also used in a test in my upcoming changes.

I haven't observed any issue with treating the bytes from write_errors as utf8.

@Imberflur Imberflur force-pushed the better-should-panic branch from 6737430 to 953f8c3 Compare December 3, 2025 16:28
Copy link
Contributor

@hazelwiss hazelwiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

…sage when converting from bauble value to rust fails, and add helper method for writing errors to a string
@Imberflur Imberflur force-pushed the better-should-panic branch from cbb99ec to 5809124 Compare December 3, 2025 17:22
/// the same.
///
/// Panics if converting object fails.
pub fn expected_from_bauble<T>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reuse this in bauble/tests/integration.rs because this takes the RwLock which I believe needs to be unlocked when calling from_bauble for certain tests (so we can't just take reference to BaubleContext here)

}

/// Writes errors and attempts to convert them to a string.
pub fn try_to_string(&self, ctx: &BaubleContext) -> Result<String, std::string::FromUtf8Error> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this can ever fail, but I wanted to keep any potential panicking out the main bauble API. So this returns a result.

Copy link
Contributor

@hazelwiss hazelwiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm still

@Imberflur Imberflur merged commit b9b716b into main Dec 4, 2025
1 check passed
@Imberflur Imberflur deleted the better-should-panic branch December 4, 2025 18:23
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.

Improve should_panic tests

3 participants