> On March 3, 2015, 3:21 p.m., Zameer Manji wrote: > > src/test/python/apache/aurora/client/cli/test_supdate.py, line 211 > > <https://reviews.apache.org/r/31710/diff/1/?file=883801#file883801line211> > > > > For the calls like this you can do > > `mock_api.pause_job_update.assert_called_once_with(self.TEST_JOBKEY)` > > instead of comparing against an array of call objects. > > Bill Farner wrote: > In case the diff didn't make it clear, i think we should settle on > `mock_calls` for uniformity. Additionally, i think the error messages from > asserting on `mock_calls` is better: > > ``` > _mock_self = <MagicMock name='ctime' id='140532173693328'>, args = (1,) > kwargs = {}, self = <MagicMock name='ctime' id='140532173693328'> > msg = 'Expected to be called once. Called 9 times.' > > > ??? > E AssertionError: Expected to be called once. Called 9 times. > ``` > > vs > > ``` > > assert self.mock_ctime.mock_calls == [call(1)] > E AssertionError: assert [call(1),\n ca...(8),\n call(9)] == > [call(1)] > E Left contains more items, first extra item: call(2) > E Full diff: > E - [call(1), > E ? ^ > E + [call(1)] > E ? ^ > E - call(2), > E - call(3), > E - call(4), > E - call(5), > E - call(6), > E - call(7), > E - call(8), > E - call(9)] > ``` > > Moreover, `assert x.mock_calls == [...]` is not vulnerable to typos. In > this test, the first typoed line passes, but the second fails. > > ``` > self.mock_ctime.assret_called_once_with(1) > assert self.mock_ctime.mock_clls == [call(n) for n in range(1, 10)] > ```
Your reasoning is sound. I would like to point out the nice error messages come from using `assert` statements under a py.test runner. - Zameer ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31710/#review75083 ----------------------------------------------------------- On March 3, 2015, 3:16 p.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31710/ > ----------------------------------------------------------- > > (Updated March 3, 2015, 3:16 p.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Bugs: AURORA-1165 and AURORA-1166 > https://issues.apache.org/jira/browse/AURORA-1165 > https://issues.apache.org/jira/browse/AURORA-1166 > > > Repository: aurora > > > Description > ------- > > Discovered these while working on AURORA-1157 > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/update.py > ee8146be0cb6e3f3139a5491c883864950f245bd > src/test/python/apache/aurora/client/cli/test_supdate.py > 1173650998d397a68a7fabf38201fb9feebf2977 > > Diff: https://reviews.apache.org/r/31710/diff/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
