[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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



On Fri, Dec 7, 2012 at 8:01 PM, Damian Johnson <atagar@xxxxxxxxxxxxxx> 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


_______________________________________________
tor-dev mailing list
tor-dev@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev