[tor-bugs] #5472 [Stem]: Stem version parser matches git hash too

[Tor Bug Tracker & Wiki]

#5472: Stem version parser matches git hash too
--------------------+-------------------------------------------------------
 Reporter:  neena   |          Owner:  neena         
     Type:  defect  |         Status:  needs_revision
 Priority:  normal  |      Milestone:                
Component:  Stem    |        Version:                
 Keywords:          |         Parent:                
   Points:          |   Actualpoints:                
--------------------+-------------------------------------------------------
Changes (by atagar):

  * status:  new => needs_revision


Comment:

 Hi neena, thanks for the patch! Looks great, only a few small changes that
 I'd suggest.

 > Treat versions with spaces in them as invalid

 At present the regex won't match spaces but it will match other
 whitespace. Maybe replace "(-[^ ]*)" with "(-\S*)"?

 > Fix Version to match . strictly

 Nice catch. Would you mind adding a unit test for this (maybe something
 like "1a2a3a4")?

 > Returns a function that passes calls on to the stem.util.system.call()
 > function if it's argument doesn't begin one of the keys in outputdict

 This is almost perfect, my only suggestion here would be to change the
 behavior when the system call isn't in outputdict. This is a unit test so,
 if we're making system calls from it, we're already doing something wrong
 (since we're making the test os dependent). Instead of passing through to
 system.call lets fail the test instead. This could look like...

 {{{
 VERSION_CALL_OUTPUT = """\
 Mar 22 23:09:37.088 [notice] Tor v0.2.2.35 (git-73ff13ab3cc9570d). \
 This is experimental software. Do not rely on it for \
 strong anonymity. (Running on Linux i686)
 Tor version 0.2.2.35 (git-73ff13ab3cc9570d)."""

 class system_call_mocker:
   def __init__(self, current_test, responses):
     self.current_test = current_test
     self.responses = responses

   def call(self, command):
     if command in responses:
       return responses[command].splitlines()
     else:
       self.current_test.fail()

 def test_get_system_tor_version(self):
   system_mocker = system_call_mocker(self, {"tor --version":
 VERSION_CALL_OUTPUT})
   mocking.mock(stem.util.system.call, system_mocker.call)
   version = stem.version.get_system_tor_version()
   self.assert_versions_match(version, 0, 2, 2, 35, None)
 }}}

 (Side Note: I didn't know about '\' for multi-line string blocks. Thanks,
 that is gonna make a few other places much more readable...)

 When you're done please put the ticket into "Needs code/patch review" so
 it's clear at a glance what this patch's state is.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5472#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online