[tor-bugs] #7713 [Stem]: Stem should invalidate GETCONF cache

[Tor Bug Tracker & Wiki]

#7713: Stem should invalidate GETCONF cache
-----------------------------+----------------------------------------------
 Reporter:  atagar           |          Owner:  atagar        
     Type:  defect           |         Status:  needs_revision
 Priority:  minor            |      Milestone:                
Component:  Stem             |        Version:                
 Keywords:  controller easy  |         Parent:                
   Points:                   |   Actualpoints:                
-----------------------------+----------------------------------------------
Changes (by atagar):

  * status:  needs_review => needs_revision


Comment:

 Hi Sreenatha. This looks good except that it caused a couple issues that
 would have been revealed by running the tests...

 1. Your patch uses tab indentations...

 {{{
 atagar at morrigan:~/Desktop/stem$ ./run_tests.py --style
 ======================================================================
                              INITIALISING
 ======================================================================

   checking stem version...                             1.0.1-dev
   checking python version...                           2.7.1
   checking pycrypto version...                         2.6
   checking pyflakes version...                         0.5.0
   checking pep8 version...                             1.4
   checking for orphaned .pyc files...                  done
   checking for unused tests...                         done

 STATIC CHECKS
 * /home/atagar/Desktop/stem/stem/control.py
   line 681  - indentation has a tab
   line 683  - indentation has a tab
   line 684  - indentation has a tab
   line 685  - indentation has a tab
   line 686  - indentation has a tab
   line 687  - indentation has a tab
   line 688  - indentation has a tab
   line 689  - indentation has a tab
   line 691  - indentation has a tab
   line 692  - indentation has a tab

 TESTING PASSED (32 seconds)
 }}}

 2. This broke our integration tests.

 {{{
 ======================================================================
 FAIL: test_getconf
 ----------------------------------------------------------------------
 Traceback:
   File "/home/atagar/Desktop/stem/test/integ/control/controller.py", line
 441, in test_getconf
     self.assertEqual([",".join(n) for n in nodefamilies],
 controller.get_conf("nodefamily", multiple = True))
 AssertionError: Lists differ: ['abc,xyz,pqrs', 'mno,tuv,wxyz... !=
 ['mno,tuv,wxyz']

 First differing element 0:
 abc,xyz,pqrs
 mno,tuv,wxyz

 First list contains 1 additional elements.
 First extra element 1:
 mno,tuv,wxyz

 - ['abc,xyz,pqrs', 'mno,tuv,wxyz']
 + ['mno,tuv,wxyz']

 ----------------------------------------------------------------------
 Ran 31 tests in 11.210s

 FAILED
 }}}

 Running './run_tests.py --all' is a good way of catching these kinds of
 issues. I've pushed a version of your patch that simply invalidates the
 cache (this avoids the bug the issue spotted by the integ test)...

 https://gitweb.torproject.org/stem.git/commitdiff/de576cad4bbdd4901ee1857334a7420593ef2458

 ... however if you wouldn't mind a couple things would make this better...

 1. Mind looking into why this integ test was failing, and revise the cache
 updating you had to account for it?

 2. It would be useful to have a unit or integ test that demonstrates the
 caching bug that this patch solves so we don't have this kind of
 regression in the future.

 Thanks! -Damian

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