Re: testsuite: prefix handling

[Pedro Alves <palves at redhat dot com>]

On 02/21/2012 06:05 PM, Tom Tromey wrote:
> 
> Pedro> Only a few tests weren't converted to use with_test_prefix by this
> Pedro> patch.  Those are ones that I found that:
> 
> Pedro>  - would require a large reindent
> Pedro>  - or with_test_prefix wasn't a 1-1 replacement
> 
> I don't mind large reindents in a situation like this.
> I think the resulting code would be better.
> It is up to you, though; I also don't mind the status quo.

I really dislike mixing in a lot of formatting changes with
code changes, so I was really avoiding it.  It's a harder patch to
read/prove right, and a harder patch to maintain as well.  If someone
sent me over this change mixed in with a lots of reindents, and hadn't
at least sent me a -W diff, I'd groan.  :-)  We can always do
a "reindent" patch as follow up, perhaps even as multiple patches.

> 
> Pedro> WDYT?
> 
> Pedro> -proc altivec_abi_tests { extra_flags force_abi } {
> Pedro> +proc altivec_abi_tests { prefix extra_flags force_abi } { with_test_prefix $prefix {
> [...]
> Pedro> +}}
> 
> I think newlines and reindentation would result in prettier code.

Honestly, I kind of liked the current formatting -- I was kind of reading it
as a function attribute.  But I don't mind seeing this re-indented either.
But that's a lower priority to me.

> Pedro> -    altivec_abi_tests "additional_flags=-maltivec" "auto"
> Pedro> +    altivec_abi_tests " default ABI, auto:" "additional_flags=-maltivec" "auto"
> 
> It would be both prettier and better encapsulation if with_test_prefix
> supplied the space.

Thing is I stumbled on tests that don't currently output a space.  So I'm not
sure we should always require spaces.  Changing those would make this patch
not an identity patch -- it'd produce different gdb.sum output compared to
the current mainline.  I can try making with_test_prefix always add the space
instead as follow up, and see which tests are those, and if we'd get something
non-sensical.  Probably not.

> 
> Pedro> +# Test files shall make sure all the test result lines in gdb.sum are
> Pedro> +# unique in a test run, so that comparing the gdb.sum files of two
> Pedro> +# test runs gives correct results.  Test files that exercise
> Pedro> +# variations of the same tests more than once, shall prefix the
> Pedro> +# different test invocations with different identifying strings in
> Pedro> +# order to make them unique.
> [...]
> 
> I love the comment.

:-)

-- 
Pedro Alves