The changes look great! I see no issues with merging to the master branch. Sorry about the reStructuredText inconvenience. I actually didn't even think of it -- I just meant to clean up the comments a bit, but I'll definitely be more careful in the future.
- Erik & Megan
On Thu, Jun 14, 2012 at 4:28 PM, Damian Johnson atagar@torproject.orgwrote:
We also updated your adaptation to our patch so that no code is repeated. This should make the function cleaner and more readable. This new code
can
be found at: https://github.com/jacthinman/Tor-Stem/blob/mocking/test/mocking.py
Ah ha, makes much more sense now - thanks. After some reflection I realized that the mocking code was more convoluted that it needs to be. What do you think of this change instead?
https://gitweb.torproject.org/user/atagar/stem.git/commitdiff/eb51eab0badf6b...
It seems to handle both of our use cases while also axing much of the special handling for builtins...
import test.mocking import time my_mock = lambda i: i test.mocking.mock(time.time, my_mock) time.time(5)
5
test.mocking.revert_mocking() time.time()
1339689308.64306
test.mocking.mock(open, my_mock) open(5)
5
test.mocking.revert_mocking() open("/some_file", "r")
Traceback (most recent call last): File "<stdin>", line 1, in <module> IOError: [Errno 2] No such file or directory: '/some_file'
If it looks good to you then I'll go ahead and merge this with master. Cheers! -Damian
PS. Thanks for converting the mocking module to reStrcutredText. Usually that would have been a welcome improvement, however in this case I'd already converted it in the master branch so that actually generated merge conflicts with your patch. It's helpful if there's separate commits for separate changes (ie, a commit for the mocking changes and another for the reStructuredText conversion). That said, I don't always follow this rule either. ;)