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

[Rémi Denis-Courmont]

Le tiistaina 1. marraskuuta 2016, 18.22.41 EET David Fuhrmann a écrit :
> I disabled a test for a distinct feature (...)

You disabled a test for necessary/relied-upon and documented behaviour of the 
API. This is wrong and needs reverting. Period.

> You are arguing that you want to use test failures to track missing
> functionality. I completely disagree with that. 

No, I did not argue that missing features should be tracked by test cases 
(although I note thast TDD pretty much consists of exactly doing that). I 
argued that removing test because true positive failures was wrong.

Meanwhile, you argued that:
"(...) tests should not be misused for new (...) functionality. There is trac 
for that. but this is what you do here"

1) I am not misusing anything and I find this statement insulting.

2) I completely disagree with the whole premise, and so do well-known industry 
best practices. Specifically, I think that test cases should be added as early 
as possible. And as a corrolary, new features would be the very best 
candidates for test cases.

3) TLS ALPN is not a "new" feature anyway, and it was not a new feature when 
the test cases for it were added either. As a matter of fat, the VLC API was 
added in August 2014, while the test cases were added in January 2016.

4) TLS ALPN is also not a "missing" feature, since all the reference TLS plug-
in support it. Whether you like it or not, the GnuTLS plug-in is the only 
reference implementation of the VLC TLS API. Historically, Gildas insisted on 
making it a plug-in only to avoid libvlc proper depending on GnuTLS, not 
really to support alternate implementations (since OpenSSL was hopeless). You 
cannot expect VLC developers at large to support an alternate implementation 
that targets a proprietary OS and a closed-source (TLS) back-end, requires 
expensive and dedicated hardware (oh, at least it is not using a different 
programming language!). Likewise VLC developers should be expected to take 
care of the Qt GUI, not the MacOS one.

Not to mention that the main, if not only, rationale for the SecureTransport 
plug-in seems to be working around the GPL, and by extension, my and other 
people´s copyright license.

In other words, MacOS/iOS stuff is the responsibility, both morally and 
practically, of the MacOS/iOS developers *only*.

> 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.)

Oh, come on. What kind of fridge logic is that?

If I filed a bug about the excessive use of __APPLE__  in the code in place of 
proper code and abtraction, you would remove (most of) them?

If I filed a bug against the SecureTransport plug-in, you would somehow be 
able and willing to make the plug-in conform to the documented VLC TLS API 
semantics, and thus add ALPN support?

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

Well, I do. That test checks for behaviour that the HTTP plug-in depends on. 
If that test is disabled, nothing protects the relevant parts of the HTTP 
plug-in from bugs and regressions.

-- 
Rémi Denis-Courmont
http://www.remlab.net/