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

Gracefully handle linebreaks in test titles #2774

Closed
novemberborn opened this issue Jun 19, 2021 · 3 comments · Fixed by #2779
Closed

Gracefully handle linebreaks in test titles #2774

novemberborn opened this issue Jun 19, 2021 · 3 comments · Fixed by #2779
Labels
breaking requires a SemVer major release bug current functionality does not work as desired help wanted
Milestone

Comments

@novemberborn
Copy link
Member

If test titles include line breaks, the snapshot reports for those tests will look a bit weird since the formatting code does not handle them. Reporter output may be broken too.

Per discussion in #2769 (comment) we could normalize whitespace in test titles at the point of declaration, which then impacts uniqueness checks, or we could do so when displaying.

My preference would actually be to do this at the point of declaration, which should be considered a breaking change given that we identify the test by its title. But that's OK since we're doing pre-releases for AVA 4 at the moment.

@KillyMXI what do you think?

@novemberborn novemberborn added bug current functionality does not work as desired help wanted breaking requires a SemVer major release labels Jun 19, 2021
@novemberborn novemberborn added this to the 4.0 milestone Jun 19, 2021
@KillyMXI
Copy link
Contributor

Yep, it is definitely better to have normalized whitespace throughout the whole test lifetime.
And it is great we can afford some breakage and not waste effort on extra graceful migration of tests that got excessive whitespace before.

@KillyMXI
Copy link
Contributor

Hmm. I may need the beforeAndAfter macro to ensure snap files have normalized titles.
Quick and dirty way - import it into /test/snapshot-tests/formatting.js from ../snapshot-workflow/helpers/macros.js.
Other way - move it to ../helpers/ and only have helper macros there.
I prefer the former for this issue. The latter touches way too many files and falls into housekeeping domain.

And it looks like there is no test for try assertions anywhere. /test/assertions/fixtures/happy-path.js mentions many assertions but not try. Again, not in scope of this issue, just thought I'd mention it.

KillyMXI added a commit to KillyMXI/ava that referenced this issue Jun 22, 2021
KillyMXI added a commit to KillyMXI/ava that referenced this issue Jun 22, 2021
@novemberborn
Copy link
Member Author

And it looks like there is no test for try assertions anywhere. /test/assertions/fixtures/happy-path.js mentions many assertions but not try. Again, not in scope of this issue, just thought I'd mention it.

I think those are tested at https://github.com/avajs/ava/blob/main/test-tap/test-try-commit.js.

novemberborn added a commit that referenced this issue Jul 4, 2021
Fixes #2774.

Co-authored-by: Mark Wubben <mark@novemberborn.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking requires a SemVer major release bug current functionality does not work as desired help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants