[tor-dev] Stem code review 2012-12-04

[Sean Robinson]

On Fri, Dec 7, 2012 at 8:01 PM, Damian Johnson <atagar at torproject.org>wrote:

> > The quoted key/value mapping is more readable, now.  Good work.  Why not
> look for quoted positional args before non-quoted positional args?  Why not
> do just like kwarg handling?
>

MY_EVENT "arg1 arg2"
>
> ... where the spec says we really *do* have two non-quoted positional
> arguments. That said, if we ever saw such a thing I'd probably
> conclude that tor was *trying* to confuse us. :P
>
> Patch welcome.
>

I must not be reasoning about Event._parse_standard_attr() correctly.  I
think it is already looking for _QUOTED positional args, but is working at
it backwards from _KEYWORD_ARGS parsing.

Well, I already wanted to write unit tests for Event.  So, I'll try
exercising the class and see what I can see.


> > Why restrict SignalEvents to expected_signals when control-spec.txt
> allows more?  This may mean changes later to add support for things the
> protocol already claims to support.
>
> It's not being restricted. The expected_signals is only used so that
> we log if we get something else. It doesn't prevent us from having
> other values - I'd just like to know if we get them since the pydocs
> and spec would then need to be tweaked.
>

While preparing to argue further, I see the mistake I made: SIGNAL command
versus SIGNAL event.  I was reading the wrong part of the spec.  Please
disregard.


> > I set up coverage.py for another project and I wondered if it would work
> with Stem.
>


> Sweet! Mind sending me a link to the coverage tool? I'd like to see
> which modules are lacking coverage.
>

http://nedbatchelder.com/code/coverage/
http://pypi.python.org/pypi/coverage

I've used this on three projects (not counting Stem) and it helps check
that written tests are reaching all the dark crevices of one's code.  If
you have questions, I'm happy to help. Once installed, do:

1) coverage run --parallel-mode --branch --omit="test*" ./run_tests.py -u
-i -t RUN_NONE (or some variation)
2) coverage combine
3) coverage html
4) read the nice reports in the htmlcov/ directory


-- 
Sean Robinson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20121208/bad2b4e0/attachment.html>