Hi Christophe,
> On Tue, 19 Feb 2019 at 10:28, Christophe Lyon
> <christophe.l...@linaro.org> wrote:
>>
>> On Mon, 18 Feb 2019 at 21:12, Rainer Orth <r...@cebitec.uni-bielefeld.de>
>> wrote:
>> >
>> > Hi Christophe,
>> >
>> > > dg-extract-results currently moves lines like
>> > > WARNING: program timed out
>> > > at the end of each .exp section when it generates .sum files.
>> > >
>> > > This is because it sorts its output based on the 2nd field, which is
>> > > normally the testname as in:
>> > > FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto
>> > > -fno-use-linker-plugin -flto-partition=none execution test
>> > >
>> > > As you can notice 'program' comes after
>> > > gcc.c-torture/execute/20020129-1.c alphabetically, and generally after
>> > > most (all?) GCC testnames.
>> > >
>> > > This is a bit of a pain when trying to handle transient test failures
>> > > because you can no longer match such a WARNING line to its FAIL
>> > > counterpart.
>> > >
>> > > The attached patch changes this behavior by replacing the line
>> > > WARNING: program timed out
>> > > with
>> > > WARNING: gcc.c-torture/execute/20020129-1.c -O2 -flto
>> > > -fno-use-linker-plugin -flto-partition=none execution test program
>> > > timed out
>> > >
>> > > The effect is that this line will now appear immediately above the
>> > > FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto
>> > > -fno-use-linker-plugin -flto-partition=none execution test
>> > > so that it's easier to match them.
>> > >
>> > >
>> > > I'm not sure how much people depend on the .sum format, I also
>> > > considered emitting
>> > > WARNING: program timed out gcc.c-torture/execute/20020129-1.c -O2
>> > > -flto -fno-use-linker-plugin -flto-partition=none execution test
>> > >
>> > > I also restricted the patch to handling only 'program timed out'
>> > > cases, to avoid breaking other things.
>> > >
>> > > I considered fixing this in Dejagnu, but it seemed more complicated,
>> > > and would delay adoption in GCC anyway.
>> > >
>> > > What do people think about this?
>> >
>> > I just had a case where your patch broke the generation of go.sum.
>> > This is on Solaris 11.5 with python 2.7.15:
>> >
>> > ro@colima 68 > /bin/ksh
>> > /vol/gcc/src/hg/trunk/local/gcc/../contrib/dg-extract-results.sh
>> > testsuite/go*/*.sum.sep > testsuite/go/go.sum
>> > Traceback (most recent call last):
>> > File "/vol/gcc/src/hg/trunk/local/gcc/../contrib/dg-extract-results.py",
>> > line 605, in <module>
>> > Prog().main()
>> > File "/vol/gcc/src/hg/trunk/local/gcc/../contrib/dg-extract-results.py",
>> > line 569, in main
>> > self.parse_file (filename, file)
>> > File "/vol/gcc/src/hg/trunk/local/gcc/../contrib/dg-extract-results.py",
>> > line 427, in parse_file
>> > num_variations)
>> > File "/vol/gcc/src/hg/trunk/local/gcc/../contrib/dg-extract-results.py",
>> > line 311, in parse_run
>> > first_key = key
>> > UnboundLocalError: local variable 'key' referenced before assignment
>> >
>> > Before your patch, key cannot have been undefined, now it is. I've
>> > verified this by removing the WARNING: lines from the two affected
>> > go.sum.sep files and now go.sum creation just works fine.
>> >
>>
>> Sorry for the breakage.
>>
>> Can you send me the .sum that cause the problem so that I can reproduce it?
>>
>
> So the problem happens when a WARNING is the first result of a new harness.
> This is fixed by the attached dg-extract-results.patch2.txt.
>
> While looking at it, I noticed that the ordering wasn't right with the
> shell version,
> though I did test it before sending the previous patch.
> The attached dg-extract-results.patch1.txt makes sure the WARNING: line
> appears before the following testcase with the shell version too.
>
> Are both OK?
ok for now to avoid silently losing considerable parts of the test
results. However, I've lots of problems with the current approach which
I'll detail in a separate reply to your original test submissions.
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
