[PATCH 0/N] test-suite improvement - PASS/FAIL: initial patch - fix

[Tomas Dohnalek]

Hi Marek,
thank you for your input.

Dne 10.9.2012 22:21, Marek Polacek napsal(a):
> On Mon, Sep 10, 2012 at 01:04:48PM -0400, Tomas Dohnalek wrote:
>> -do-tests-clean:
>> +tests-summary = $(common-objpfx)tests.sum
>> +tests-summary-clean:
>> +	rm -f $(tests-summary)
>> +
> You probably want to use `-rm' here.

True.

>> diff --git a/Rules b/Rules
>> index 17d938e..5a569dd 100644
>> --- a/Rules
>> +++ b/Rules
>> @@ -127,6 +127,8 @@ binaries-shared-tests = $(filter-out $(binaries-pie) $(binaries-static), \
>>   				     $(binaries-all-tests))
>>   binaries-shared-notests = $(filter-out $(binaries-pie) $(binaries-static), \
>>   				       $(binaries-all-notests))
>> +test-name = $(subdir)/$(*F)
> Can't we use just $(@F) here?

That will produce "PASS: tst-empty.out" instead of nicer and more explaining "PASS: csu/tst-empty". 
I think, that there is a need to distinguish tests from different directories.

>> +test_rc=$1
>> +test_name=$2
>> +
>> +if [ $# -gt 2 ]; then
>> +    output=$3
>> +else
>> +    output=/dev/stdout
>> +fi
> Is this hunk really needed?  That is, couldn't we just drop this and
> then in Makefiles just do
> $(evaluate-test) > $(tests-summary)
> Anyway, it looks like we always output into $(tests-summary)...

Ok, it is more straightforward, I tried to write it more universally, but there probably wont be no 
need for that functionality.

>> +
>> +if [ ${test_rc} -eq 0 ]; then
>> +    result="PASS"
>> +else
>> +    result="FAIL"
>> +fi
> Use 2 spaces.  In the future we probably will want to have
> SKIP/UNSUPPORTED option too, I suspect.
>
>> +echo "${result}: ${test_name}" >> ${output}
>> +exit ${test_rc}
> There's no need to use ${var}.  Just $var is good enough.  Thanks,

I prefer to use it this way, but if $var is glibc convention, I have no problem with that.

> Furthermore, I'm not sure about parallel make...

True, but Joseph S. Myers proposal seems to be dealing with parallelism, I will make new patch.

> 	Marek

Thanks
Tomas

-- 
Tomas Dohnalek
QE Tools Intern
Red Hat