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