Re: testsuite: prefix handling

Pedro Alves
2012-02-21T19:54:26Z

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