[MPlayer-dev-eng] [PATCH] Add fate test

[Diego Biurrun]

On Wed, Nov 09, 2011 at 08:30:13AM +0100, Reimar Döffinger wrote:
> On 9 Nov 2011, at 02:43, Diego Biurrun <diego at biurrun.de> wrote:
> > On Sun, Nov 06, 2011 at 11:04:10AM +0100, Reimar Döffinger wrote:
> >> a bit hackish, video only, some samples fail even when testing on a
> >> single configuration.
> > 
> >> --- Makefile    (revision 34309)
> >> +++ Makefile    (working copy)
> >> @@ -926,6 +926,7 @@
> >> 
> >> clean:
> >>    -$(MAKE) -C ffmpeg $@
> >> +    -$(MAKE) -C tests clean
> >>    -rm -f $(call ADD_ALL_DIRS,/*.o /*.a /*.ho /*~)
> >>    -rm -f $(call ADD_ALL_EXESUFS,mplayer mencoder)
> >> 
> >> @@ -1087,12 +1088,14 @@
> >> 
> >> +fatetest: mplayer$(EXESUF)
> >> +    $(MAKE) -C tests fatetest
> >> 
> >> 
> >> -include $(DEP_FILES) $(DRIVER_DEP_FILES) $(TESTS_DEP_FILES) $(TOOLS_DEP_FILES) $(DHAHELPER_DEP_FILES)
> >> 
> >> .PHONY: all doxygen *install* *tools drivers dhahelper*
> >> -.PHONY: checkheaders *clean tests check_checksums
> >> +.PHONY: checkheaders *clean tests check_checksums fatetest
> > 
> > I designed the build system non-recursive and monolithic on purpose.
> > 
> > tests/Makefile should be part of the main Makefile.
>
> Well, including it should be no problem, but I really don't want
> everything in one single Makefile, that one is already almost as
> "nice" to get an overview of as mplayer.c

Currently the Makefile is 1134 lines, but only 400 lines or so of that
are actually code, the rest is variable declarations with one file per
line.  Given the size and complexity of MPlayer this is very moderate.

Notice that the most arcane stuff in there comes from VIDIX, kernel
drivers, tools and other things of little relevance nowadays.  If you
are worried about build system complexity, let's eliminate some more
cruft.

> I remember you complaining that people don't apply good programming
> principles to build systems, so this is a good point at applying the
> principle "independent stuff I different files" IMO.

If only it were independent - it is not.  Recursive make is always a bad
idea.  I'm under the impression that you have not read the "Recursive
Make Considered Harmful" paper yet.  Go right ahead, it's short and
sweet:

http://miller.emu.id.au/pmiller/books/rmch/

Folding tests/Makefile into the top-level Makefile would not only add
less lines than tests/Makefile has now; it would also actually work.

> There's also some other minor annoyance like that it is convenient
> to run the test script from the tests directory, that's a bit more
> difficult with non-recursive make.

Just shake the habit ..

> >> --- tests/faterun.sh    (revision 0)
> >> +++ tests/faterun.sh    (revision 0)
> >> @@ -0,0 +1,10 @@
> >> +#!/bin/sh
> >> +i=$1
> >> +echo "running $i"
> >> +mkdir -p res/$(dirname $i)
> >> +touch res/$i.md5
> >> +../mplayer -noconfig all -lavdopts threads=4:bitexact -really-quiet -noconsolecontrols -nosound -benchmark -vo md5sum:outfile=res/$i.md5 $FATE_SAMPLES/$i
> >> +if ! diff -uw ref/$i.md5 res/$i.md5 ; then
> >> +    mv res/$i.md5 res/$i.md5.bad
> >> +    exit 1
> >> +fi
> > 
> > "i" is not really a better variable name than "1".
> > Also, a few empty lines could make this more readable.
> 
> I had a loop in there originally and forgot to clean it up.
> I is still better since it's easier to change i=$1 to i=$2 for example
> than having to change it everywhere.

Nah, I'm operating under the assumption that everybody is smart enough
for automatic search and replace around here :)

Diego