Convert FetcherTest to use mock instead of mox.

Review Request #4142 — Created Aug. 7, 2016 and submitted

jsirois
pants
jsirois/fetcher/use_mock
3324, 3759
4099
4143
37299a6...
pants-reviews
kwlzn, stuhood, zundel
tests/python/pants_test/net/http/BUILD           |   2 +-
 tests/python/pants_test/net/http/test_fetcher.py | 224 +++++++++++++++++++++++++++-----------------------
 2 files changed, 123 insertions(+), 103 deletions(-)
Locally green `./pants test tests/python/pants_test/net/http`.

CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/150791491
JS
  1. Stu and Eric are on as original reviewers of https://rbcommons.com/s/twitter/r/4099/ and I added Kris for his mock expertise.

  2. 
      
JS
KW
  1. lgtm - one note around mock.Mock vs mock.create_autospec.

  2. tests/python/pants_test/net/http/test_fetcher.py (Diff revision 1)
     
     
     
     
     

    I prefer to use mock.create_autospec(X) wherever possible in these cases for full autospec'ing, which I believe nets deeper mock<->real object parity.

    ideally you'd also use spec_set=True as well, but that may not always be feasible.

    1. OK - thanks for that pointer - it got me reading more widely. The spec_set documentation in Mock is much worse ( should be s/on the object passed as `spec_set`/on the object passed as `spec`/) than the documentation of the same parameter in create_autospec!
      I was able to use create_autospec for the Fetcher.Listener class (with the baffling but simple in the adjustment of only using assert_called_with... methods in certain cases within context manager blocks where mock methods were 1st created).
      I was not able to use create_autpspec at all for the requests classes since they both establish their attributes in __init__.

  3. 
      
ST
  1. Thanks John.

  2. 
      
JS
JS
KW
  1. Ship It!
  2. 
      
JS
Review request changed

Status: Closed (submitted)

Change Summary:

Now on master:

git log -1 origin/master
commit b5eb940cf734a1c379f46051a67bd76895f756b6
Author: John Sirois <john.sirois@gmail.com>
Date:   Tue Aug 9 09:24:42 2016 -0600

    Convert FetcherTest to use mock instead of mox.
    
    Testing Done:
    Locally green `./pants test tests/python/pants_test/net/http`.
    
    CI went green here:
      https://travis-ci.org/pantsbuild/pants/builds/150791491
    
    Bugs closed: 3324, 3759
    
    Reviewed at https://rbcommons.com/s/twitter/r/4142/
Loading...