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

[Reimar Döffinger]

On Wed, Nov 09, 2011 at 03:27:10PM +0100, Diego Biurrun wrote:
> On Wed, Nov 09, 2011 at 08:30:13AM +0100, Reimar Döffinger wrote:
> > 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/

That one suggests splitting make files and including them, which is
exactly what I was arguing for here, so I do not follow your point.
(though not putting it into the subdir and/or not calling it Makefile
would be advisable).
However it misses things that with including you still end up with a
single variable name-space, so at least in that aspect you _still_ need
to know the whole Makefile including everything it includes.
I also slightly disagree with you line number argument: If you're new
to a build system and need to change something the total number of lines
matters almost as much as how many actually "do something useful".
Btw. the median file size of .c files seems to be around 350 lines, so
even by that measure the Makefile is larger.
Either way you either make sure I know a way to fix it that you'd accept
or could you consider doing it yourself?
I could just go ahead, but I am too lazy to do something and risk you'll
hate it :-P

> > >> --- 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 :)

That doesn't make the diff better.
So a proper variable name I would consider the better solution.