On 10/20/2015 01:49 PM, Lukas Slebodnik wrote:
On (20/10/15 12:09), Nikolai Kondrashov wrote:
> On 10/19/2015 11:36 AM, Lukas Slebodnik wrote:
>> On (02/10/15 19:15), Nikolai Kondrashov wrote:
>>> Hi everyone,
>>>
>>> I've found an embarrassing logic failure in my Valgrind execution design
in
>>> CI: tests which were excluded from Valgrind checks weren't running at
all,
>>> instead of just running without Valgrind.
>>>
>>> The first attached patch fixes that. The second patch makes CI handle test
>>> executables which start with a dash. Not that we're likely to ever have
any,
>>> but just in case.
>>>
>>> Sorry for overlooking this.
>>>
>>> Nick
>>
>> >From 6bd14712d9d6d8297f6356d65bb3db040dae7a3a Mon Sep 17 00:00:00 2001
>>> From: Nikolai Kondrashov <Nikolai.Kondrashov(a)redhat.com>
>>> Date: Fri, 2 Oct 2015 19:02:55 +0300
>>> Subject: [PATCH 1/2] CI: Do not skip tests not checked with Valgrind
>>>
>>> Make contrib/ci/valgrind-condense execute programs not matching the
>>> supplied PATH_PATTERN without Valgrind, instead of simply exiting
>>> successfully.
>>>
>>> This makes the make-check-valgrind stage actually run the tests not
>>> checked with Valgrind, instead of skipping them.
>>
>> Could you exaplain the purpose of this patch?
>>
>> Because te dlopen_test and whitespace_test were executed as part of
>> make-check-valgrind.
>>
>> sh$ contrib/ci/run -n
>> autoreconf: success 00:00:14 ci-autoreconf.log
>> DEBUG BUILD: ci-build-debug
>> configure: success 00:00:13 ci-build-debug/ci-configure.log
>> make-tests: success 00:02:26 ci-build-debug/ci-make-tests.log
>> make-check-valgrind: success 00:01:29
ci-build-debug/ci-make-check-valgrind.log
>> SUCCESS
>>
>> sh$ cat ci-build-debug/dlopen-tests.log
>> PASS dlopen-tests (exit status: 0)
>>
>> sh$ cat ./ci-build-debug/src/tests/whitespace_test.log
>> PASS src/tests/whitespace_test (exit status: 0)
>
> Valgrind-condense wraps individual tests and is invoked by automake test
> harness as a wrapper for each test. The test harness thinks the test has
> executed and prints the "PASS ..." message above, whereas in fact it's
only
> valgrind-condense which doesn't do anything (exits immediately, without
> running the test), because the test name didn't match the pattern.
>
> The patch makes valgrind-condense run the test without Valgrind instead of
> just exiting.
>
ahh,
I finally noticed that there was missing text in log file about PASSED
and FAILED tests.
It should be:
$ cat dlopen-tests.log
Running suite(s): dlopen
100%: Checks: 1, Failures: 0, Errors: 0
PASS dlopen-tests (exit status: 0
But it was only:
sh$ cat ci-build-debug/dlopen-tests.log
PASS dlopen-tests (exit status: 0)
It is not huge problem because we did not plan to execute
these test with valgrind anyway. And these missing tests were
executed as part of make distcheck or in mock build.
Ah, glad to hear nobody's going to kill me for this :)
However, I thought it was a typical practice (at least a while ago) to run
essential test set before pushing, which has Valgrind only.
ACK
Thanks for the review, Lukas!
Nick