[vlc-devel] [vlc-commits] tests/tls: Disable checks for ALPN on apple platforms

[Steve Lhomme]

On Tue, Nov 1, 2016 at 6:22 PM, David Fuhrmann <david.fuhrmann at gmail.com> wrote:
>
> Am 01.11.2016 um 16:09 schrieb Rémi Denis-Courmont <remi at remlab.net>:
>
> Le tiistaina 1. marraskuuta 2016, 15.12.29 EET David Fuhrmann a écrit :
>
> vlc | branch: master | David Fuhrmann <dfuhrmann at videolan.org> | Tue Nov  1
> 14:35:08 2016 +0100| [784ab6ce693675fb554060d1e1f8d194c354e3ff] |
> committer: David Fuhrmann
>
> tests/tls: Disable checks for ALPN on apple platforms
>
>
> Disabling tests because of a true positive failure? You have got to be
> kidding. Even a fresh IT graduate would know better than that.
>
>
> Hello,
>
> I disabled a test for a distinct feature which is not implemented in the
> securetransport module and thus not available on all Apple platforms for its
> default configuration. This feature will not be added in the near future as
> the underlying library just does not support that.
>
> You are arguing that you want to use test failures to track missing
> functionality. I completely disagree with that. For such kind of things, we
> have trac. If you want to have a feature in, create a trac ticket and set
> the priority to high, if its important. (Which I believe is not in this
> case.) But do not misuse tests.
> But if the whole team sees that differently, I’m happy to hear your
> arguments.

While running tests on Windows (on which I have to disable 50% of them
to see what may actually break when doing a change) I noticed the
tests have a "skip" feature. Although I never quite understood how it
works. I think this should fall under this category when on macOS this
test should be skipped. And it's mentioned in the "make check" logs.

I also think it should depend on the OS X version. The current one
doesn't support it but when the test is run on the next version an
error might occur and we can verify the status of the feature or it's
supported and no further action needs to be done.

> Seeing that, I see no problem with disabling ALPN related tests.
>
>
> On Apple platforms, ALPN does not work as securetransport does
> not provide any public API for that. So do not check for that
> feature until support is added.
>
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=784ab6ce693675fb554
> 060d1e1f8d194c354e3ff
>
> ---
>
> test/modules/misc/tls.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/test/modules/misc/tls.c b/test/modules/misc/tls.c
> index 60898c6..a2ddbf0 100644
> --- a/test/modules/misc/tls.c
> +++ b/test/modules/misc/tls.c
> @@ -201,9 +201,13 @@ int main(void)
>     answer = 1;
>     val = securepair(&th, &tls, alpnv, &alp);
>     assert(val == 0);
> +
> +    /* SecureTransport, used on apple platforms, does not support ALPN */
> +#ifndef __APPLE__
>     assert(alp != NULL);
>     assert(!strcmp(alp, "bar"));
>     free(alp);
> +#endif
>
>
> Oh and not only it removes a valid test cases, but it obviously introduces a
> leak if GnuTLS is used (which is perfectly possible on MacOS).
>
>
> Indeed, I’ll fix that.
>
> BR. David
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>