[tor-dev] #6512 [Stem]: TorExport Module and Unit Testing

[Erik I Islo]

Hi Damian,

Looking at coverage the test provides, I was under the impression that the
various cases of include_fields and exclude fields (e.g. if they overlap)
should also be tested, but if you find that unnecessary, that would help
explain why my unit tests were difficult to follow.

In the mocking changes that I made, I added functionality for mocking
functions that take keyword arguments.  Though it is no longer necessary
with the new unit tests, I was wondering if there was something I could
have done better with that code as it seems such functionality may come in
useful in the future.

You may have already noticed this (and your documentation is consistent
with this), but just to be sure, the new export code won't work with
generators, which I bring up as this is what server_descriptor.parse_file()
returns.  So a user of this module would need to covert that generator into
a list or tuple, which could be rather large.

>>> def gener(list=[1,2,3,4]):
...   for item in list:
...     yield item

>>> gener()[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'generator' object has no attribute '__getitem__'

Working with Professor Danner this summer (and from the CS classes I have
taken so far), I am under the impression that run-time type checking can be
expensive and generally falls outside of the realm of 'pythonic' code.  To
help Megan and I understand this design change, could you explain why you
went with this style (e.g. line 83 in stem/descriptor/export.py)?  Along
similar lines, why wouldn't we also check the types of other parameters
passed into the function?

Thank you in advance for answering these questions -- this summer has been
(and still is) a phenomenal learning experience for us, and gaining insight
on coding style and the like is crucial to developing our own coding
techniques.

Looking at the code, I must apologize.  I didn't mean to take so much of
your time rewriting the code.  That said, looking at your implementation
gave me some perspective in evaluating the code I submitted so I don't make
similar mistakes in the future.

Best,
Erik

On Sat, Aug 4, 2012 at 3:22 PM, Tor Bug Tracker & Wiki <
torproject-admin at torproject.org> wrote:

> #6512: TorExport Module and Unit Testing
>
> ----------------------------+-----------------------------------------------
>     Reporter:  ErikI        |       Owner:  atagar
>         Type:  enhancement  |      Status:  closed
>     Priority:  normal       |   Milestone:
>    Component:  Stem         |     Version:
>   Resolution:  implemented  |    Keywords:
>       Parent:               |      Points:
> Actualpoints:               |
>
> ----------------------------+-----------------------------------------------
> Changes (by atagar):
>
>   * status:  needs_review => closed
>   * resolution:  => implemented
>
>
> Comment:
>
>  Thanks Erik, merged with some revisions [1]. I found the unit tests a bit
>  hard to follow, especially since it wasn't broken down by the test cases
>  so I rewrote those. Let me know if I missed any test cases that you think
>  are important.
>
>  [1]
>
> https://gitweb.torproject.org/stem.git/commitdiff/7022021c4207a0066cb93261629f2ba020d307f6
>
> --
> Ticket URL: <
> https://trac.torproject.org/projects/tor/ticket/6512#comment:3>
> Tor Bug Tracker & Wiki <https://trac.torproject.org/>
> The Tor Project: anonymity online
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20120806/d3fb9520/attachment.html>