Re: [PATCHv2 0/3] Automatic detection of test name problems

Simon Marchi
2020-04-29T18:23:00Z

On 2020-04-29 12:03 p.m., Keith Seitz wrote:
> On 4/29/20 8:38 AM, Andrew Burgess wrote:
>> * Simon Marchi <simark@simark.ca> [2020-04-29 11:04:40 -0400]:
>>
>>> When you detect an offender, do you think you could print something? A bit like a "FAIL"
>>> is printed when a test fails. For example:
>>>
>>> DUPLICATE: gdb.base/break.exp: set convenience variable $foo to 81.5
>>>
>>> That would make it easier to spot the problematic test(s). But even without that, your
>>> patchset looks good and useful to me.
>
> That is a great idea, Simon. [Again -- why didn't I think of that?]
>
>> However, I chose to print "DUPLICATE: ...." for _every_ duplicate, so
>> you would get this:
>>
>> PASS: foo
>> PASS: foo
>> DUPLICATE: foo
>> PASS: foo
>> DUPLICATE: foo
>> PASS: bar
>> PASS: bar
>> DUPLICATE: bar
>> PASS: bar
>> DUPLICATE: bar
>>
>> Obviously we don't get a DUPLICATE message for the first 'foo', or the
>> first 'bar', we can't know by that point that these are going to be
>> duplicated.
>>
>> But, it might be confusing that this test will report 2 duplicates,
>> but contain 4 DUPLICATE lines. Would it in fact be better to report a
>> count of 4 in this case I wonder?
>
> I don't really care one way or the other. This is an exceptional case, and
> is /far/ less offensive than, e.g., assertions aborting gdb. Seeing messages
> just reinforces the message IMO. YMMV.

I also don't mind, as long as it pops in my face. It could be printed only once
and it would be enough.

This looks good to me:

DUPLICATE: gdb.base/break.exp: set convenience variable $foo to 81.5
DUPLICATE: gdb.base/break.exp: set breakpoint via non-integer convenience variable disallowed

Simon