aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joshua Cohen" <jco...@twopensource.com>
Subject Re: Review Request 26363: Make the large-update check in the client update command consider instance parameters.
Date Wed, 08 Oct 2014 22:21:37 GMT


> On Oct. 6, 2014, 4:44 p.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/client/cli/test_update.py, lines 342-353
> > <https://reviews.apache.org/r/26363/diff/1/?file=714123#file714123line342>
> >
> >     This sequence of mocks and writing config to a file is repeated in... many (all?)
of these tests. Can you refactor to remove the repetition?
> 
> Mark Chu-Carroll wrote:
>     I really think that that should not be done.
>     
>     For tests, I really like to be able to see, in an instant, exactly what mocks are
being used by a particular test.  I don't want to have to look somewhere else; I don't want
to have to mentally combine the stuff that's mocked in a common frame and the different stuff
that's mocked and/or reconfigured in a particular test. The test should be as clear as it
can be, and anything from the environment that it's mucking with should be done explicitly
in the most local-possible context.

Not a ship blocker for me, just my perspective, but these mocks are identical in several calls,
if many client test cases need to mock the exact same set of things in the exact same way,
it seems worthwhile to simplify that process.


- Joshua


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


On Oct. 8, 2014, 5:40 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26363/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2014, 5:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-792
>     https://issues.apache.org/jira/browse/aurora-792
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make the large-update check in the client update command consider instance parameters.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/context.py 301531fcb443297facb78d87a18073c8b7fd4064

>   src/main/python/apache/aurora/client/cli/jobs.py 103fb53cb5101d3e025d8712e3887bccdfbb6aeb

>   src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8

>   src/test/python/apache/aurora/client/cli/test_update.py 85b1db19d89967a741bfba7964eeb368426f0b61

> 
> Diff: https://reviews.apache.org/r/26363/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added, all test pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message