denoland / std

The Deno Standard Library
https://jsr.io/@std
MIT License
3k stars 598 forks source link

refactor(assert): align additional error messages #5784

Closed irbull closed 2 weeks ago

irbull commented 3 weeks ago

Aligns the error messages in the assert folder to match the style guide.

https://github.com/denoland/std/issues/5574

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.25%. Comparing base (bb0db58) to head (e3bf6ad). Report is 11 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #5784 +/- ## ========================================== + Coverage 96.24% 96.25% +0.01% ========================================== Files 481 481 Lines 38746 38738 -8 Branches 5617 5615 -2 ========================================== - Hits 37290 37288 -2 + Misses 1415 1409 -6 Partials 41 41 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kt3k commented 2 weeks ago

This will break some of Deno CLI's snapshot testing as they asserts the entire assertion error output (like this one: https://github.com/denoland/deno/blob/d54d29662f30c0fa5e1f048fdce4835e51248682/tests/specs/test/hide_stacktraces/dot.out )

I'm not sure at this point how this can be disruptive to the ecosystem in a similar way as Deno CLI repo.

Maybe this needs more discussion/investigation before landing.

irbull commented 2 weeks ago

@kt3k Good catch, and that's fair. While I like the idea of consistent error messages I think we need to be pragmatic too. I don't mind looking through those assertions and trying to update them, but that doesn't help the broader community if they too rely on specific assertions in their tests.

irbull commented 2 weeks ago

Here is an alternative way of handling this: https://github.com/denoland/std/pull/5797

kt3k commented 2 weeks ago

5797 makes sense to me. Closing this one for now