> On Nov. 19, 2014, 11:08 p.m., Joshua Cohen wrote:
> > I'm not sold on the idea of adding explicit injection points for 
> > dependencies rather than mocking them out. I feel like these are a lot of 
> > hoops to jump through for the sake of testability leading to more code 
> > that's more difficult to maintain. I don't really have a solution to the 
> > problem of mock.patch mutating global state (making concurrent test 
> > execution potentially flaky) other than not running tests concurrently. I'm 
> > not sure which is more cumbersome to live with ;).

You know what. This is a really bad argument on my part ;). I've actually got 
no objections to avoiding mock.patch, my main concern in the context of this 
review is the need to extract another method to test due to @app.command 
shenanigans, that's unfortunate, but it shouldn't cloud the larger discussion.

Consider my objections lifted.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28250/#review62266
-----------------------------------------------------------


On Nov. 19, 2014, 8:44 p.m., Kevin Sweeney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28250/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 8:44 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Zameer 
> Manji.
> 
> 
> Bugs: AURORA-938
>     https://issues.apache.org/jira/browse/AURORA-938
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Fix test_version to use explicit injection points.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/commands/core.py 
> 852b6b1ba479a8781133f2331fb75f8122a36855 
>   src/test/python/apache/aurora/client/commands/test_version.py 
> 5c3c45a769aeec2ffb2c2bd3d8d937476c9abd12 
> 
> Diff: https://reviews.apache.org/r/28250/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python:all
> ./pants src/main/python/apache/aurora/client/bin:aurora_client
> dist/aurora_client.pex version
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Reply via email to