[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_review
 Priority:  normal  |      Milestone:              
Component:  Stem    |        Version:              
 Keywords:          |         Parent:              
   Points:          |   Actualpoints:              
--------------------+-------------------------------------------------------
Changes (by neena):

  * status:  needs_revision => needs_review


Comment:

 Replying to [comment:5 atagar]:
 > > responses is a dictionary of prefix:output pairs. prefix should be a
 string and
 > > output should either be a string-like object or a list of strings.
 >
 > Minor thing but please replace this comment with the method for
 documenting arguments used throughout the codebase...
 Sorry, put it off for later and forgot. Won't happen again.

 > On reflection this is probably a little more than we need. For this test
 we just want two things...
 Agreed.

 >
 > 1. for system.call to return that response
 > 2. for 'something failure-ish' to happen if system.call is not called
 with 'tor --version'
 >
 > We don't need prefix matching, str/list responses, or anything else
 that's fancy (I'm assuming that you copied this from the system integ
 tests where these features were needed).
 Yeap :)

 >
 > a. Simplify this mocking to be specific to the test, this would make the
 test...
 >
 > {{{
 > def test_get_system_tor_version(self):
 >   def _mock_call(command):
 >     if command == "tor --version":
 >       return <stuff>
 >     else:
 >       raise ValueError("system.call received an unexpected command: %s"
 % command)
 >
 >   mocking.mock(stem.util.system.call, _mock_call)
 >   ... rest of the test
 > }}}
 >
 We won't be able to reuse the function for different outputs if we do
 this.

 > b. Alternatively we could reuse the system test's call mocking function
 by moving it to 'test.mocking'. However, I favor option 'a' until we have
 a test that needs to do that style of mocking on 'system.call'.
 >
 The mocking function in system is (kinda) the opposite of this.
 Paraphrased from the docstring, it passes calls on to
 stem.util.system.call if it matches a prefix, acts as a no-op otherwise.
 We'll have to have both anyway.

 c. Using a lambda to do this. It triggers the OSError in
 get_system_tor_version with ("'%s' didn't have any output" % version_cmd)
 on failure. If that happens, it could effectively be thought of as mocking
 the OSError that would've been raised if this had not been a unit test? :p

 {{{
   def test_get_system_tor_version(self):
     def _call_mocker(resp):
       return lambda cmd: resp.splitlines() if cmd == "tor --version" else
 False

     mocking.mock(stem.util.system.call, _call_mocker(
     """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)."""))
     version = stem.version.get_system_tor_version()
     self.assert_versions_match(version, 0, 2, 2, 35, None)
 }}}

 (response becomes resp because of the 80-character rule. Too neat to
 split)

 > Sorry about having so much back-and-forth. :)
 Well, I'm feeling really good about my code now. :)

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