----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/#review134373 -----------------------------------------------------------
Looking much better! A few minor suggestions. src/main/python/apache/aurora/client/cli/jobs.py (lines 171 - 172) <https://reviews.apache.org/r/47550/#comment199166> If the sole purpose of this method is unit test facilitation you can rather use mock patch instead. src/main/python/apache/aurora/client/cli/jobs.py (line 212) <https://reviews.apache.org/r/47550/#comment199167> You can import DiffFormatter directly: ``` from apache.aurora.client.cli.diff_formatter import DiffFormatter ``` src/main/python/apache/aurora/client/cli/update.py (lines 162 - 163) <https://reviews.apache.org/r/47550/#comment199169> Same as above, use mock patch instead. src/main/python/apache/aurora/client/cli/update.py (line 177) <https://reviews.apache.org/r/47550/#comment199168> Same as above, import directly. src/test/python/apache/aurora/client/cli/test_diff.py (line 38) <https://reviews.apache.org/r/47550/#comment199162> Use 'spec=DiffFormatter' for a stronger assertion of mocked functionality. src/test/python/apache/aurora/client/cli/test_diff_formatter.py (lines 56 - 58) <https://reviews.apache.org/r/47550/#comment199160> You can use it directly from AuroraClientCommandTest. Also, to shorten the access points you may want to extend from AuroraClientCommandTest rather than from unittest.TestCase. src/test/python/apache/aurora/client/cli/test_diff_formatter.py (lines 60 - 62) <https://reviews.apache.org/r/47550/#comment199161> This is no longer needed if you reference the above from AuroraClientCommandTest. src/test/python/apache/aurora/client/cli/test_diff_formatter.py (lines 113 - 115) <https://reviews.apache.org/r/47550/#comment199158> This seems to be used in only one place, suggest inlining. src/test/python/apache/aurora/client/cli/test_diff_formatter.py (line 118) <https://reviews.apache.org/r/47550/#comment199159> Same here src/test/python/apache/aurora/client/cli/test_supdate.py (line 100) <https://reviews.apache.org/r/47550/#comment199157> Use 'spec=DiffFormatter' for a stronger assertion of mocked functionality. - Maxim Khutornenko On May 20, 2016, 10:24 p.m., Mehrdad Nurolahzade wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47550/ > ----------------------------------------------------------- > > (Updated May 20, 2016, 10:24 p.m.) > > > Review request for Aurora. > > > Repository: aurora > > > Description > ------- > > AURORA-1492 Improve "aurora update start" command output > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION > src/main/python/apache/aurora/client/cli/jobs.py > e8bc38aaff42579419130b116389d9b9e09122a9 > src/main/python/apache/aurora/client/cli/update.py > eb7e9b0799bb25af652a0d5bde231cfca5bc1510 > src/test/python/apache/aurora/client/cli/test_diff.py > b9e91cf3be6ebd6d99e588ba2a2df304e42fd832 > src/test/python/apache/aurora/client/cli/test_diff_formatter.py > PRE-CREATION > src/test/python/apache/aurora/client/cli/test_supdate.py > 2135ca988ce2f260262145a2168849acd8d14874 > > Diff: https://reviews.apache.org/r/47550/diff/ > > > Testing > ------- > > Automated: ./pants test.pytest --no-fast src/test/python:: > Manual: using vagrant > e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > Style: ./build-support/hooks/pre-commit > > Example output: > > aurora update start devcluster/www-data/prod/hello > ./aurora/examples/jobs/updated_hello_world.aurora > INFO] Starting update for: hello > Job update has started. View your update progress at > http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b > This job update will: > add instances: [2-3] > update instances: [0-1] > > > Thanks, > > Mehrdad Nurolahzade > >
