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