Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publish the assert_prints spec helper #13599

Merged
merged 5 commits into from
Jun 27, 2023

Conversation

HertzDevil
Copy link
Contributor

This does not expose string_build_via_utf16 because it was only needed by spec/manual/string_normalize_spec.cr to define a helper similar to assert_prints that uses a custom expectation; instead, assert_prints itself is overloaded to accept expectations that are not eq.

@straight-shoota
Copy link
Member

I think exposing this is a good idea. User code will need to do similar specs as stdlib.

I'm not sure about the should overload, though. Is it really useful? It seems tailored to an exotic use case which would not be made much use of outside of stdlib.

@HertzDevil
Copy link
Contributor Author

The expectation could also be match or contain, I don't think they are that rare in specs. But more than that we shouldn't encourage people to copy the body of assert_prints and swap out just the expectation if they do need one that isn't eq

@straight-shoota straight-shoota modified the milestone: 1.9.0 Jun 23, 2023
@HertzDevil
Copy link
Contributor Author

This is a tough one. #13539 means that the IO overloads use eq(%str.scrub) and the other ones use eq(%str). Thus there is no longer a single expectation used throughout assert_prints.

Specs outside String itself should never have to deal with invalid encodings, so I think we should revert the scrub added there and express those specs in some other way.

@straight-shoota straight-shoota added this to the 1.9.0 milestone Jun 26, 2023
@straight-shoota straight-shoota changed the title Publish the assert_prints helper Publish the assert_prints spec helper Jun 27, 2023
@straight-shoota straight-shoota merged commit f692397 into crystal-lang:master Jun 27, 2023
50 checks passed
@HertzDevil HertzDevil deleted the feature/assert_prints branch June 29, 2023 14:44
Blacksmoke16 pushed a commit to Blacksmoke16/crystal that referenced this pull request Dec 11, 2023
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants