[gnutls-devel] valgrind-tests vs full-test-suite

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[gnutls-devel] valgrind-tests vs full-test-suite

Alon Bar-Lev-3
Hi,

I believe this is the last issue in tests of gnutls-3.5... :)

I started stabilization process in Gentoo, it will be nice if we can
solve this nicely as well.

Look at this full-test-suite[1] conditional:
---
# test if we are in git master or in release build. In release
# builds we do not use valgrind.
SUITE_FILE="${srcdir}/tests/suite/mini-eagain2.c"
if test "$full_test_suite" = yes && test ! -f "$SUITE_FILE";then
    full_test_suite=no
    VALGRIND=""
    AC_SUBST(VALGRIND, [])
    opt_valgrind_tests=no
fi
---

This actually disables valgrind in non-git but I do not see any reason
to do so as valgrind is supported in some other tests. If I disable
the full-tests-suite then I can enjoy valgrind extra tests.

Any reason why we condition this?

Thanks!
Alon

[1] https://gitlab.com/gnutls/gnutls/blob/master/configure.ac#L360

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Nikos Mavrogiannopoulos
On Mon, Mar 13, 2017 at 6:49 PM, Alon Bar-Lev <[hidden email]> wrote:

> Hi,
>
> I believe this is the last issue in tests of gnutls-3.5... :)
>
> I started stabilization process in Gentoo, it will be nice if we can
> solve this nicely as well.
>
> Look at this full-test-suite[1] conditional:
> ---
> # test if we are in git master or in release build. In release
> # builds we do not use valgrind.
> SUITE_FILE="${srcdir}/tests/suite/mini-eagain2.c"
> if test "$full_test_suite" = yes && test ! -f "$SUITE_FILE";then
>     full_test_suite=no
>     VALGRIND=""
>     AC_SUBST(VALGRIND, [])
>     opt_valgrind_tests=no
> fi
> ---
>
> This actually disables valgrind in non-git but I do not see any reason
> to do so as valgrind is supported in some other tests. If I disable
> the full-tests-suite then I can enjoy valgrind extra tests.
> Any reason why we condition this?

If I remember well, the reason this was introduced was to avoid test
suite failures due to leaks or other issues in unrelated libraries on
the released version. E.g., if you try to compile the latest release
of gnutls in a system which has a libc with a leak, you wouldn't have
the test suite fail because of that.

regards,
Nikos

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Alon Bar-Lev-3
On 14 March 2017 at 09:36, Nikos Mavrogiannopoulos <[hidden email]> wrote:

> On Mon, Mar 13, 2017 at 6:49 PM, Alon Bar-Lev <[hidden email]> wrote:
>> Hi,
>>
>> I believe this is the last issue in tests of gnutls-3.5... :)
>>
>> I started stabilization process in Gentoo, it will be nice if we can
>> solve this nicely as well.
>>
>> Look at this full-test-suite[1] conditional:
>> ---
>> # test if we are in git master or in release build. In release
>> # builds we do not use valgrind.
>> SUITE_FILE="${srcdir}/tests/suite/mini-eagain2.c"
>> if test "$full_test_suite" = yes && test ! -f "$SUITE_FILE";then
>>     full_test_suite=no
>>     VALGRIND=""
>>     AC_SUBST(VALGRIND, [])
>>     opt_valgrind_tests=no
>> fi
>> ---
>>
>> This actually disables valgrind in non-git but I do not see any reason
>> to do so as valgrind is supported in some other tests. If I disable
>> the full-tests-suite then I can enjoy valgrind extra tests.
>> Any reason why we condition this?
>
> If I remember well, the reason this was introduced was to avoid test
> suite failures due to leaks or other issues in unrelated libraries on
> the released version. E.g., if you try to compile the latest release
> of gnutls in a system which has a libc with a leak, you wouldn't have
> the test suite fail because of that.

The valgrind tests may be enabled or disabled.
In release tarball there is no full-suite.
The result is that valgrind cannot be enabled unless the non-existence
full-suite is disabled...
So I do not think this logic is required.

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Nikos Mavrogiannopoulos
On Tue, Mar 14, 2017 at 8:41 AM, Alon Bar-Lev <[hidden email]> wrote:

>>> This actually disables valgrind in non-git but I do not see any reason
>>> to do so as valgrind is supported in some other tests. If I disable
>>> the full-tests-suite then I can enjoy valgrind extra tests.
>>> Any reason why we condition this?
>>
>> If I remember well, the reason this was introduced was to avoid test
>> suite failures due to leaks or other issues in unrelated libraries on
>> the released version. E.g., if you try to compile the latest release
>> of gnutls in a system which has a libc with a leak, you wouldn't have
>> the test suite fail because of that.
>
> The valgrind tests may be enabled or disabled.

That's true but we have them enabled by default, so builds would fail
for that reason and that would create more noise in the list.

> In release tarball there is no full-suite.
> The result is that valgrind cannot be enabled unless the non-existence
> full-suite is disabled...
> So I do not think this logic is required.

That's true, but I am afraid of the issue above (when this was added
was specifically to avoid such reports from the list). It should be
combined with another change that makes valgrind not to run by default
on releases.

regards,
Nikos

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Alon Bar-Lev-3
On 14 March 2017 at 14:41, Nikos Mavrogiannopoulos <[hidden email]> wrote:

>
> On Tue, Mar 14, 2017 at 8:41 AM, Alon Bar-Lev <[hidden email]> wrote:
>
> >>> This actually disables valgrind in non-git but I do not see any reason
> >>> to do so as valgrind is supported in some other tests. If I disable
> >>> the full-tests-suite then I can enjoy valgrind extra tests.
> >>> Any reason why we condition this?
> >>
> >> If I remember well, the reason this was introduced was to avoid test
> >> suite failures due to leaks or other issues in unrelated libraries on
> >> the released version. E.g., if you try to compile the latest release
> >> of gnutls in a system which has a libc with a leak, you wouldn't have
> >> the test suite fail because of that.
> >
> > The valgrind tests may be enabled or disabled.
>
> That's true but we have them enabled by default, so builds would fail
> for that reason and that would create more noise in the list.
>
> > In release tarball there is no full-suite.
> > The result is that valgrind cannot be enabled unless the non-existence
> > full-suite is disabled...
> > So I do not think this logic is required.
>
> That's true, but I am afraid of the issue above (when this was added
> was specifically to avoid such reports from the list). It should be
> combined with another change that makes valgrind not to run by default
> on releases.

Hi,
Can we disable this by default and enable it explicitly in all ci jobs?
Or can we add another flag that is disabled by default and when
enabled will not touch the VALGRIND?
Alon

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Alon Bar-Lev-3
On 14 March 2017 at 15:01, Alon Bar-Lev <[hidden email]> wrote:

> On 14 March 2017 at 14:41, Nikos Mavrogiannopoulos <[hidden email]> wrote:
>>
>> On Tue, Mar 14, 2017 at 8:41 AM, Alon Bar-Lev <[hidden email]> wrote:
>>
>> >>> This actually disables valgrind in non-git but I do not see any reason
>> >>> to do so as valgrind is supported in some other tests. If I disable
>> >>> the full-tests-suite then I can enjoy valgrind extra tests.
>> >>> Any reason why we condition this?
>> >>
>> >> If I remember well, the reason this was introduced was to avoid test
>> >> suite failures due to leaks or other issues in unrelated libraries on
>> >> the released version. E.g., if you try to compile the latest release
>> >> of gnutls in a system which has a libc with a leak, you wouldn't have
>> >> the test suite fail because of that.
>> >
>> > The valgrind tests may be enabled or disabled.
>>
>> That's true but we have them enabled by default, so builds would fail
>> for that reason and that would create more noise in the list.
>>
>> > In release tarball there is no full-suite.
>> > The result is that valgrind cannot be enabled unless the non-existence
>> > full-suite is disabled...
>> > So I do not think this logic is required.
>>
>> That's true, but I am afraid of the issue above (when this was added
>> was specifically to avoid such reports from the list). It should be
>> combined with another change that makes valgrind not to run by default
>> on releases.
>
> Hi,
> Can we disable this by default and enable it explicitly in all ci jobs?
> Or can we add another flag that is disabled by default and when
> enabled will not touch the VALGRIND?
> Alon

I can disable these by default if it is a release tarball (no
full-test-suite), maybe this is the best.

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Nikos Mavrogiannopoulos
In reply to this post by Alon Bar-Lev-3
On Tue, Mar 14, 2017 at 2:01 PM, Alon Bar-Lev <[hidden email]> wrote:

>> That's true but we have them enabled by default, so builds would fail
>> for that reason and that would create more noise in the list.
>>
>> > In release tarball there is no full-suite.
>> > The result is that valgrind cannot be enabled unless the non-existence
>> > full-suite is disabled...
>> > So I do not think this logic is required.
>>
>> That's true, but I am afraid of the issue above (when this was added
>> was specifically to avoid such reports from the list). It should be
>> combined with another change that makes valgrind not to run by default
>> on releases.

> Can we disable this by default and enable it explicitly in all ci jobs?
> Or can we add another flag that is disabled by default and when
> enabled will not touch the VALGRIND?

Makes sense.

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Alon Bar-Lev-3
On 14 March 2017 at 15:34, Nikos Mavrogiannopoulos <[hidden email]> wrote:

> On Tue, Mar 14, 2017 at 2:01 PM, Alon Bar-Lev <[hidden email]> wrote:
>>> That's true but we have them enabled by default, so builds would fail
>>> for that reason and that would create more noise in the list.
>>>
>>> > In release tarball there is no full-suite.
>>> > The result is that valgrind cannot be enabled unless the non-existence
>>> > full-suite is disabled...
>>> > So I do not think this logic is required.
>>>
>>> That's true, but I am afraid of the issue above (when this was added
>>> was specifically to avoid such reports from the list). It should be
>>> combined with another change that makes valgrind not to run by default
>>> on releases.
>
>> Can we disable this by default and enable it explicitly in all ci jobs?
>> Or can we add another flag that is disabled by default and when
>> enabled will not touch the VALGRIND?
>
> Makes sense.

Hmmm... I probably did not understand what option "Makes sense" :)
So I've done the first...[1], please checkout.
I found that much more CI jobs explicitly disables valgrind anyway...
so it actually makes everything simpler.
I can do the second if you do not like this one.

[1] https://gitlab.com/gnutls/gnutls/merge_requests/309

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [gnutls-devel] valgrind-tests vs full-test-suite

Nikos Mavrogiannopoulos
On Tue, 2017-03-14 at 22:27 +0200, Alon Bar-Lev wrote:

> On 14 March 2017 at 15:34, Nikos Mavrogiannopoulos <[hidden email]>
> wrote:
> > On Tue, Mar 14, 2017 at 2:01 PM, Alon Bar-Lev <[hidden email]
> > m> wrote:
> > > > That's true but we have them enabled by default, so builds
> > > > would fail
> > > > for that reason and that would create more noise in the list.
> > > >
> > > > > In release tarball there is no full-suite.
> > > > > The result is that valgrind cannot be enabled unless the non-
> > > > > existence
> > > > > full-suite is disabled...
> > > > > So I do not think this logic is required.
> > > >
> > > > That's true, but I am afraid of the issue above (when this was
> > > > added
> > > > was specifically to avoid such reports from the list). It
> > > > should be
> > > > combined with another change that makes valgrind not to run by
> > > > default
> > > > on releases.
> > > Can we disable this by default and enable it explicitly in all ci
> > > jobs?
> > > Or can we add another flag that is disabled by default and when
> > > enabled will not touch the VALGRIND?
> >
> > Makes sense.
>
> Hmmm... I probably did not understand what option "Makes sense" :)

Sorry, I meant the first one: "Can we disable this by default and
enable it explicitly in all ci jobs?"

> So I've done the first...[1], please checkout.

Thanks, commented on the MR.

regards,
Nikos


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Loading...