Hi Damian,<div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><div><div>>>> def gener(list=[1,2,3,4]):</div><div>... for item in list:</div><div>... yield item</div><div><br></div><div>>>> gener()[0]</div><div>Traceback (most recent call last):</div>
<div> File "<stdin>", line 1, in <module></div><div>TypeError: 'generator' object has no attribute '__getitem__'</div></div><div><br></div><div>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?</div>
<div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><div>Best,</div><div>Erik<br><br><div class="gmail_quote">On Sat, Aug 4, 2012 at 3:22 PM, Tor Bug Tracker & Wiki <span dir="ltr"><<a href="mailto:torproject-admin@torproject.org" target="_blank">torproject-admin@torproject.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>#6512: TorExport Module and Unit Testing<br>
</div>----------------------------+-----------------------------------------------<br>
Reporter: ErikI | Owner: atagar<br>
Type: enhancement | Status: closed<br>
<div> Priority: normal | Milestone:<br>
Component: Stem | Version:<br>
</div> Resolution: implemented | Keywords:<br>
Parent: | Points:<br>
Actualpoints: |<br>
----------------------------+-----------------------------------------------<br>
Changes (by atagar):<br>
<br>
* status: needs_review => closed<br>
* resolution: => implemented<br>
<br>
<br>
Comment:<br>
<br>
Thanks Erik, merged with some revisions [1]. I found the unit tests a bit<br>
hard to follow, especially since it wasn't broken down by the test cases<br>
so I rewrote those. Let me know if I missed any test cases that you think<br>
are important.<br>
<br>
[1]<br>
<a href="https://gitweb.torproject.org/stem.git/commitdiff/7022021c4207a0066cb93261629f2ba020d307f6" target="_blank">https://gitweb.torproject.org/stem.git/commitdiff/7022021c4207a0066cb93261629f2ba020d307f6</a><br>
<span><font color="#888888"><br>
--<br>
Ticket URL: <<a href="https://trac.torproject.org/projects/tor/ticket/6512#comment:3" target="_blank">https://trac.torproject.org/projects/tor/ticket/6512#comment:3</a>><br>
</font></span><div><div>Tor Bug Tracker & Wiki <<a href="https://trac.torproject.org/" target="_blank">https://trac.torproject.org/</a>><br>
The Tor Project: anonymity online<br>
</div></div></blockquote></div><br></div>