Re: Review Request 26137: Fix help for new update command.
On Sept. 29, 2014, 11:32 p.m., Joe Smith wrote: src/main/python/apache/aurora/client/cli/update.py, line 45 https://reviews.apache.org/r/26137/diff/1/?file=708198#file708198line45 Could you update a test case to catch accessing these as properties to catch accidental regressions? David McLaughlin wrote: Piggy backing this issue to add that my ship it is pending a test for this command at least? Mark Chu-Carroll wrote: I don't know of any way to write a single test that would always catch this. Joe Smith wrote: Rebased off your diff: [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff diff --git a/src/main/python/apache/aurora/client/cli/update.py b/src/main/python/apache/aurora/client/cli/update.py index 41475a7..ef07a11 100644 --- a/src/main/python/apache/aurora/client/cli/update.py +++ b/src/main/python/apache/aurora/client/cli/update.py @@ -42,7 +42,6 @@ class StartUpdate(Verb): INSTANCES_SPEC_ARGUMENT, CONFIG_ARGUMENT ] - @property def help(self): return textwrap.dedent(\ Start a scheduler-driven rolling upgrade on a running job, using the update diff --git a/src/test/python/apache/aurora/client/cli/test_update.py b/src/test/python/apache/aurora/client/cli/test_update.py index eeed774..1a38ffe 100644 --- a/src/test/python/apache/aurora/client/cli/test_update.py +++ b/src/test/python/apache/aurora/client/cli/test_update.py @@ -23,6 +23,7 @@ from apache.aurora.client.api.quota_check import QuotaCheck from apache.aurora.client.api.scheduler_mux import SchedulerMux from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION, EXIT_OK from apache.aurora.client.cli.client import AuroraCommandLine +from apache.aurora.client.cli.update import StartUpdate from apache.aurora.client.cli.util import AuroraClientCommandTest, FakeAuroraCommandContext, IOMock from apache.aurora.config import AuroraConfig @@ -301,6 +302,10 @@ class TestUpdateCommand(AuroraClientCommandTest): 'Update completed successfully'] assert mock_err.get() == [] + def test_update_start_help(self): +start = StartUpdate() +assert 'Start a scheduler-driven rolling' in start.help + @classmethod def assert_correct_addinstance_calls(cls, api): assert api.addInstances.call_count == 20 [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ ./pants ./src/test/python/apache/aurora/client/cli:job Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, job)]) test session starts = platform darwin -- Python 2.6.8 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 61 items src/test/python/apache/aurora/client/cli/test_cancel_update.py .. src/test/python/apache/aurora/client/cli/test_create.py .. src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py src/test/python/apache/aurora/client/cli/test_open.py . src/test/python/apache/aurora/client/cli/test_restart.py src/test/python/apache/aurora/client/cli/test_status.py ... src/test/python/apache/aurora/client/cli/test_update.py ..F... == FAILURES == __ TestUpdateCommand.test_update_start_help __ self = test_update.TestUpdateCommand testMethod=test_update_start_help def test_update_start_help(self): start = StartUpdate() assert 'Start a scheduler-driven rolling' in start.help E TypeError: argument of type 'instancemethod' is not iterable src/test/python/apache/aurora/client/cli/test_update.py:307: TypeError
Re: Review Request 26137: Fix help for new update command.
On Sept. 30, 2014, 2:32 a.m., Joe Smith wrote: src/main/python/apache/aurora/client/cli/update.py, line 45 https://reviews.apache.org/r/26137/diff/1/?file=708198#file708198line45 Could you update a test case to catch accessing these as properties to catch accidental regressions? David McLaughlin wrote: Piggy backing this issue to add that my ship it is pending a test for this command at least? I don't know of any way to write a single test that would always catch this. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review54953 --- On Sept. 29, 2014, 11:05 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/ --- (Updated Sept. 29, 2014, 11:05 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-748 https://issues.apache.org/jira/browse/aurora-748 Repository: aurora Description --- Fix help for new update command. Diffs - src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 Diff: https://reviews.apache.org/r/26137/diff/ Testing --- manual testing. Thanks, Mark Chu-Carroll
Re: Review Request 26137: Fix help for new update command.
On Sept. 29, 2014, 11:32 p.m., Joe Smith wrote: src/main/python/apache/aurora/client/cli/update.py, line 45 https://reviews.apache.org/r/26137/diff/1/?file=708198#file708198line45 Could you update a test case to catch accessing these as properties to catch accidental regressions? David McLaughlin wrote: Piggy backing this issue to add that my ship it is pending a test for this command at least? Mark Chu-Carroll wrote: I don't know of any way to write a single test that would always catch this. Rebased off your diff: [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff diff --git a/src/main/python/apache/aurora/client/cli/update.py b/src/main/python/apache/aurora/client/cli/update.py index 41475a7..ef07a11 100644 --- a/src/main/python/apache/aurora/client/cli/update.py +++ b/src/main/python/apache/aurora/client/cli/update.py @@ -42,7 +42,6 @@ class StartUpdate(Verb): INSTANCES_SPEC_ARGUMENT, CONFIG_ARGUMENT ] - @property def help(self): return textwrap.dedent(\ Start a scheduler-driven rolling upgrade on a running job, using the update diff --git a/src/test/python/apache/aurora/client/cli/test_update.py b/src/test/python/apache/aurora/client/cli/test_update.py index eeed774..1a38ffe 100644 --- a/src/test/python/apache/aurora/client/cli/test_update.py +++ b/src/test/python/apache/aurora/client/cli/test_update.py @@ -23,6 +23,7 @@ from apache.aurora.client.api.quota_check import QuotaCheck from apache.aurora.client.api.scheduler_mux import SchedulerMux from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION, EXIT_OK from apache.aurora.client.cli.client import AuroraCommandLine +from apache.aurora.client.cli.update import StartUpdate from apache.aurora.client.cli.util import AuroraClientCommandTest, FakeAuroraCommandContext, IOMock from apache.aurora.config import AuroraConfig @@ -301,6 +302,10 @@ class TestUpdateCommand(AuroraClientCommandTest): 'Update completed successfully'] assert mock_err.get() == [] + def test_update_start_help(self): +start = StartUpdate() +assert 'Start a scheduler-driven rolling' in start.help + @classmethod def assert_correct_addinstance_calls(cls, api): assert api.addInstances.call_count == 20 [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ ./pants ./src/test/python/apache/aurora/client/cli:job Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, job)]) test session starts = platform darwin -- Python 2.6.8 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 61 items src/test/python/apache/aurora/client/cli/test_cancel_update.py .. src/test/python/apache/aurora/client/cli/test_create.py .. src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py src/test/python/apache/aurora/client/cli/test_open.py . src/test/python/apache/aurora/client/cli/test_restart.py src/test/python/apache/aurora/client/cli/test_status.py ... src/test/python/apache/aurora/client/cli/test_update.py ..F... == FAILURES == __ TestUpdateCommand.test_update_start_help __ self = test_update.TestUpdateCommand testMethod=test_update_start_help def test_update_start_help(self): start = StartUpdate() assert 'Start a scheduler-driven rolling' in start.help E TypeError: argument of type 'instancemethod' is not iterable src/test/python/apache/aurora/client/cli/test_update.py:307: TypeError 1 failed, 60 passed in 6.07 seconds = src.test.python.apache.aurora.client.cli.job . FAILURE [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff diff --git
Re: Review Request 26137: Fix help for new update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review55125 --- Ship it! Ship It! - David McLaughlin On Sept. 29, 2014, 3:05 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/ --- (Updated Sept. 29, 2014, 3:05 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-748 https://issues.apache.org/jira/browse/aurora-748 Repository: aurora Description --- Fix help for new update command. Diffs - src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 Diff: https://reviews.apache.org/r/26137/diff/ Testing --- manual testing. Thanks, Mark Chu-Carroll
Re: Review Request 26137: Fix help for new update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review54953 --- src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/26137/#comment95276 Could you update a test case to catch accessing these as properties to catch accidental regressions? - Joe Smith On Sept. 29, 2014, 8:05 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/ --- (Updated Sept. 29, 2014, 8:05 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-748 https://issues.apache.org/jira/browse/aurora-748 Repository: aurora Description --- Fix help for new update command. Diffs - src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 Diff: https://reviews.apache.org/r/26137/diff/ Testing --- manual testing. Thanks, Mark Chu-Carroll
Review Request 26137: Fix help for new update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/ --- Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-748 https://issues.apache.org/jira/browse/aurora-748 Repository: aurora Description --- Fix help for new update command. Diffs - src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 Diff: https://reviews.apache.org/r/26137/diff/ Testing --- manual testing. Thanks, Mark Chu-Carroll
Re: Review Request 26137: Fix help for new update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review54894 --- Ship it! Ship It! - Zameer Manji On Sept. 29, 2014, 8:05 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/ --- (Updated Sept. 29, 2014, 8:05 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-748 https://issues.apache.org/jira/browse/aurora-748 Repository: aurora Description --- Fix help for new update command. Diffs - src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 Diff: https://reviews.apache.org/r/26137/diff/ Testing --- manual testing. Thanks, Mark Chu-Carroll
