avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Gracefully handle linebreaks in test titles #2774

Closed novemberborn closed 3 years ago

novemberborn commented 3 years ago

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 https://github.com/avajs/ava/issues/2769#issuecomment-862664084 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?

KillyMXI commented 3 years ago

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 commented 3 years ago

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.

novemberborn commented 3 years ago

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.