On May 3, 2014, at 5:20 AM, exar...@twistedmatrix.com wrote:
> On 1 May, 07:23 pm, gl...@twistedmatrix.com wrote:
>>
>> On Apr 30, 2014, at 5:21 AM, exar...@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?
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
