search

groupon / assertive

Assertive is a terse yet expressive assertion library
BSD 3-Clause "New" or "Revised" License
21 stars 11 forks source link

remove abbreviations from error messages #20

Closed EndangeredMassa closed 6 years ago

EndangeredMassa commented 9 years ago

I've only ever found this feature to be frustrating. I'd rather we remove it entirely, but I'd like to hear others' thoughts on it.

jkrems commented 9 years ago

Caused some frustration for me as well. That said: the lower impact change would be to at least actually abbreviate the message, instead of swallowing it completely. E.g. turn too long of a string into too l[...] instead of the not-very-helpful string[23]. But your solution is simpler - so... :+1: from me.

EndangeredMassa commented 9 years ago

@johan should weigh in as well, I think.

johan commented 9 years ago

Drat; this PR has sailed below my notification radar. I think I'd prefer @jkrems' take, so you can do things like match or equal comparisons (for instance) on two gigantic things, without output exploding in the ci result on assertion failure.

Expected: /bacon/i
to match: string String[length: 30025]

is definitely sub-ideal in this use case, and something like:

Expected: /bacon/i
Actually: "a very insanely ...[truncated]... long thing[total length: 30025]"

would be a major improvement. (FWIW, it seems current equal comparisons don't even have the existing abbreviate behaviour, so it's even more flawed in its existing form than I thought.)

Thoughts; close this, or try for something like it (with or without changes to equal)?

EndangeredMassa commented 9 years ago

I appreciate the goal of maintaining a legible log, but truncation of any sort destroys information I may need to resolve the issue. I'd rather have a noisy log than one that doesn't have the information I need to act.

jkrems commented 9 years ago

This build failure is a great reason for removing the abbreviation: https://github.com/groupon/testium/pull/165 / https://travis-ci.org/groupon/testium/jobs/76649194#L454