Re: [MERGE] build-test: fixed err detection and added multi-test support

> On 03/10/2009 09:50 PM, Amos Jeffries wrote:
>> I applied your merge and ran a bunch of tests for the the things which
>> stood out most in my memory as problems I expected from it.
>>
>> Several of the tests I remember the testbed being unable to catch work
>> under your match. There are two that stand out though:
>>
>> 1) adding "choke me" as syntax error in config.h
>>
>> The changes you make do not pass the exit value from buildtest.sh to
>> test-builds.sh. Everything I have read on portable shell result
>> passing is contrary to this working.
>> Yet it does. Any ideas why?
> I am not a shell expert and am not really sure what you mean here. If
> the calling program (test-builds.sh) cannot use the $? variable to get
> the result of the previous call, then we can rewrite those lines using
> an if-statement. The primary reason I did not use an if-statement is
> because the current code contains two nearly identical calling
> statements. I do not know whether those are warranted, but hopefully
> they can be merged into one.

The two calls are a bit of a hack, to cope with that out-of-tree dist call
to test-builds.sh.
If we can find a better way to detect the actual code location than that
currently used to build ${top} we should be able to remove one of them.

>
> If the callee (buildtest.sh) needs an explicit exit call of some sort to
> be more portable, let's add that.

Agreed. The fix will be easier when using the merged make parameters. So
theres the excuse for that :).

>
>> (yes explicitly passing the result value back will 'fix' your patch if
>> it turns out to be an issue on some system.)
> Yes, I am sure there is a way.
>
> My understanding is that your #1 test case works fine, but you think the
> code can be improved. Please let me know if you need help with that.

Not quite. it works, I love that. But don't understand why and that
worries me a little.

>
>
>> 2) adding ChokeMe to the translated languages list.
>>
>> Results in the "Internal error" case with exit 0.
>>
>> This occurs because the (optional and manually maintained) errors grep
>> locates po2html displaying fatal warnings in the log. Make itself is
>> happy, the po2html tool is broken regarding our needed exit values.
>>
>> Something to think about is how many of the app tools do we use in
>> Makefiles and makefile helper scripts, don't return exit 0/1 the way
>> we need?
> If there are bugs in Makefiles or in the scripts they are calling, those
> bugs should be fixed. Clearly, it is wrong that make process does not
> notice a po2html error. This problem is unrelated to the test-builds
> script and may cause bad side-effects even if we never run that script.
>
> Meanwhile, the test-builds script can rely on the _secondary_ error
> detection via error grepping. That secondary check was in my patch, but
> perhaps the error pattern needs another update. Again, this should be
> treated as a workaround for bugs that exist elsewhere and not a
> test-builds script design flaw.
>
> My understanding is that your #2 test case does not work because po2html
> or Makefile is broken. We should fix the bug but can work around it if
> we update the error pattern in test-builds.sh. Please let me know if you
> need help with that.

Well, my point was that the test-builds and child need to have a reliable
way to catch these types of fubar. As your point earlier using grep is not
great, but provides a mostly usable hack.

I have lodged a bug with the Pootle project about their app. We will need
to check our own scripts/* directory.

I'm removing my reject. please merge when ready.

Amos
Received on Wed Mar 11 2009 - 23:59:02 MDT