> 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 > >
