[Twisted-Python] Some comments regarding #5190 - ``RFC 6125 ("Service Identity") implementation´´

[Glyph]

On May 3, 2014, at 5:20 AM, exarkun at twistedmatrix.com wrote:

> On 1 May, 07:23 pm, glyph at twistedmatrix.com wrote:
>> 
>> On Apr 30, 2014, at 5:21 AM, exarkun at twistedmatrix.com wrote:
>>> I've just noticed that the changeset for #5190 included some untested code.  Specifically, there are no tests for the code which detects missing dependencies and emits warnings about them.
>> 
>> My bad.  Well, technically hawkowl's bad; hawkowl is a committer and did the review and therefore has all the criminal liability in this case, but as the author who wrote the code I bear some responsibility, at least in some abstract, hypothetical sense ;-).
> 
> My hope is that by drawing attention to examples of this kind of mistake will help us avoid making the mistake in the future.  Considering what my email prompted you to write, I think it may work. :)

Mission accomplished, I guess :-).  Please continue doing so.  I wish that every commit to trunk would prompt a thread on this list, really.  Despite the epically ridiculous amount of email I get, this list is still a bit too low-traffic for my taste.

>> Thanks for working on the fix; it looks like the relevant ticket is <https://twistedmatrix.com/trac/ticket/7097>.  I'll try to review that as soon as it's ready; let me know.
> 
> No problem.  I probably should have started my previous email with thanks to you and hawkowl for working on that feature.  It is *really* good to have service identity checking support in Twisted.

Thanks for saying so.

>> The problem with code like this is that, in some configurations, it is in fact reported as covered by coverage.py.  It requires manual examination to get the intersection of a diff and a coverage report, and even when you do, we still have too many places where it's "okay" to skip coverage.
> 
> This is true - but I'm not sure the code in this case is particularly special.  It's nearly always possible to write code and tests such that coverage.py says your code is covered but without actually having any meaningful test coverage of the implementation.  After all, coverage.py only knows that a line ran or didn't.
> 
> The problem of platform- or environment-specific code requiring multiple branches which can never always all run is an extra challenge but I think a widely applicable solution is to not do that.  To add to your comments below, if there is platform- or environment-specific code then parameterize it on the environment and write tests for all of the cases.

I think what really makes this an extra challenge is that we (well, all Python programmers, really) have a smattering of different idioms for cases like this and we don't have a succinct way of expressing the optional dependency both in the import, the implementation, and the tests.

>> So until someone has a month to spend on an all-singing all-dancing combined ratcheting coverage report for all the builders and a fantastic visualization for its output which highlights every possible coverage issue, here are some specific suggestions which might avoid some parts of this class of error:
> 
> I don't think we even have a plan for a tool that will report whether a change introduces code that isn't *really* tested (contrast "tested" with "executed").

Indeed not.  So we need some ideas in the meanwhile :-).

> I think this may be an area where we do actually need to rely on people doing a good job. Perhaps to counter balance that we need to eliminate more of the other crap involved in the development process?  For example, if reviewers never had to spend any time thinking about whether the whitespace in a change was correct, they would have that much more brain power to apply to assessing the quality of the test suite.

Speaking of thanking people for things, we should also thank richard wall, david reid, you, and hawkowl for maintenance of twistedchecker.  It's a lot better than it was :).

> Thanks!  These are great suggestions.  Can we record them in a way that lets all Twisted contributors learn from this case (instead of only the people reading this thread) - but without adding to the already unreasonably large quantity of text new contributors are ostensibly already responsible for reading?

Those sound like diametrically opposed ideas :-).

> How's the unified Contributing-to-Twisted documentation effort coming?

Yeah, how is that coming?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20140503/f3abe6a0/attachment-0002.html>