Clarify that a test failed

[levitte]

Checklist

• tests are added or updated

[dot-asm]

Suggestion was rather ERROR: (type) 'expression' failed, i.e. more human-friendly. To me ERROR: (type) failed: expression is as human-unfriendly as original, just mechanic collection of words. It would have been a little bit better if not for (type) prior failed:.

[levitte]

The reason for failed: placed like that is to prevent the readability problem of quotes in quotes. Examples:

TEST_char_eq(c, 'v') 
TEST_str_eq(s, "foo") 

[dot-asm]

readability problem of quotes in quotes

But does it justify ERROR: (char) failed: c == 'v'. Is (char) failed: readable?

[levitte]

I'm biased, so I'll let someone else answer your question

[richsalz]

The only output that's generated is failure messages. I think the trade-offs of common infrastructure, many more things being checked (e.g., malloc failures), and making things easier for the future are strong arguments in favor of this. (And also for #3324). But I recognize that Andy does more work in assembler and porting to strange platforms then the rest of the time. But but, I wonder if he'll really have a problem understanding the test failure messages now.

So overall, I'm in favor. I hope andy will come around, but unless he explicitly minus-one's this, I approve.

[dot-asm]

My point is that errors are [supposed to be] rare, i.e. you have to imagine how would it appear to you say 1/2 year later when you've forgotten all about this PR. Would you appreciate if you have to struggle remembering what does given combination of words mean? Once again, are computers for people or vice versa? This has nothing to do with what I do. ERROR: (type) failed: ... is not any better than ERROR: (type) ..., so as far as I'm concerned it can as well be omitted...

[levitte]

ERROR: (type) failed: ... is not any better than ERROR: (type) ..., so as far as I'm concerned it can as well be omitted...

I disagree. However, I just pushed another commit that outputs things like you suggested. It means that we get output like this for some of the character tests:

    # ERROR: (char) ''x' < 'i'' failed @ ../master/test/test_test.c:89
    # [x] compared to [i]

[dot-asm]

You put me into impossible position. You disagree and have Rich on your side, then why change? I naturally in favour, but now it's up to Rich to decide...

[Signing off for today, sorry.]

[levitte]

I specifically disagree that the added failed: makes no difference. I remade the output because I don't necessarely disagree with your point about the look of the output, I just saw a quote problem. Two different points.

If anyone else is fine with the quoted output, I'm fine with this look.

[richsalz]

+1
Andy, we try to make sure everyone on the team is in agreement, even though the rules only require one or two approvals :)

[paulidale]

Losing the (type) or putting it at the end of the line not the start would also be an option.

[levitte]

Merged. 4f2a569

(darn, momentary lapse, I didn't add @richsalz as reviewer... unfortunately, since we don't allow fastforwarding in our repo, this can't be undone)

[richsalz]

I can live with the anonymity :)

[levitte]

Okie