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

[David Fuhrmann]

> 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 <mailto: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.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161101/d26e36c2/attachment.html>