[FFmpeg-devel] [PATCH v2] fate: Add a target for listing failed tests

[Martin Storsjö]

On Thu, 5 Dec 2024, Alexander Strasser via ffmpeg-devel wrote:

> Hi Martin,
>
> still looks good to me.
>
> On 2024-12-04 16:08 +0200, Martin Storsjö wrote:
> [...]
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -313,6 +313,12 @@ $(FATE): $(FATE_UTILS:%=tests/%$(HOSTEXESUF)) | $(FATE_OUTDIRS)
>>  fate-list:
>>  	@printf '%s\n' $(sort $(FATE))
>>
>> +fate-list-failing:
>> +	@awk -F: '$$2 != 0 { print "fate-" $$1 }' tests/data/fate/*.rep
>
> Unfortunately there is a small problem when calling
> fate-list-failing target when no .rep files are present.
>
> The target fails because we cannot open `tests/data/fate/*.rep`.
> No output on stdout, which is correct because no test failed,
> but we get error messages on stderr and a non-zero exit status.
>
> This was also present in v1, but I just spotted it only today.
>
> For now I found this way to solve it:
>
>    @! ls tests/data/fate/*.rep >/dev/null 2>&1 || awk -F: '$$2 != 0 { print "fate-" $$1 }' tests/data/fate/*.rep
>
> It's a bit expensive, but I couldn't think of something better so far.
> Maybe it's good enough for now if there is no better idea?
> Could be improved later on if needed.

Hmm, that does indeed feel a bit clunky, but I guess it might be as good 
as it gets.

Another thing that came to mind would be something on the level of make 
itself, e.g.

fate-list-failing:
ifneq (, $(wildcard tests/data/fate/*.rep))
         @awk -F: '$$2 != 0 { print "fate-" $$1 }' tests/data/fate/*.rep
endif

That works and is slightly less clunky, but it has the downside that make 
will inspect the wildcard on every invocation. The performance implication 
probably isn't measurable, but it still feels suboptimal.

I guess your suggestion is the most reasonable of them after all, but 
let's wait for another day to see if something else comes to mind.

> I think fate-clear-results is better, because it more accurately 
> describes what it does.
> 
> Maybe fate-clear-reports would be even more precise. That is if I recall 
> correctly that the `.rep` suffix was chosen as an abbrevation for 
> report.

Thanks, I guess fate-clear-reports would work as well, I can change my 
local patch to that form.

// Martin