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

[Simon Marchi <simark@simark.ca>]

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