Return-Path: X-Original-To: apmail-aurora-reviews-archive@minotaur.apache.org Delivered-To: apmail-aurora-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 8A29F10543 for ; Wed, 15 Jan 2014 21:56:45 +0000 (UTC) Received: (qmail 50829 invoked by uid 500); 15 Jan 2014 21:56:45 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 50802 invoked by uid 500); 15 Jan 2014 21:56:44 -0000 Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.incubator.apache.org Delivered-To: mailing list reviews@aurora.incubator.apache.org Received: (qmail 50793 invoked by uid 99); 15 Jan 2014 21:56:44 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 15 Jan 2014 21:56:44 +0000 X-ASF-Spam-Status: No, hits=-1997.9 required=5.0 tests=ALL_TRUSTED,HTML_MESSAGE,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Wed, 15 Jan 2014 21:56:42 +0000 Received: (qmail 50626 invoked by uid 99); 15 Jan 2014 21:56:22 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 15 Jan 2014 21:56:22 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 2E2801D435C; Wed, 15 Jan 2014 21:56:21 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============5818325462882290082==" MIME-Version: 1.0 Subject: Re: Review Request 16919: Fix bad merge commit From: "Mark Chu-Carroll" To: "Mark Chu-Carroll" Cc: "Aurora" , "Brian Wickman" Date: Wed, 15 Jan 2014 21:56:21 -0000 Message-ID: <20140115215621.26497.82750@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Mark Chu-Carroll" X-ReviewGroup: Aurora X-ReviewRequest-URL: https://reviews.apache.org/r/16919/ X-Sender: "Mark Chu-Carroll" References: <20140115215247.26497.38736@reviews.apache.org> In-Reply-To: <20140115215247.26497.38736@reviews.apache.org> Reply-To: "Mark Chu-Carroll" X-ReviewRequest-Repository: aurora X-Virus-Checked: Checked by ClamAV on apache.org --===============5818325462882290082== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16919/#review31940 ----------------------------------------------------------- Ship it! Ship It! - Mark Chu-Carroll On Jan. 15, 2014, 4:52 p.m., Brian Wickman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16919/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2014, 4:52 p.m.) > > > Review request for Aurora and Mark Chu-Carroll. > > > Repository: aurora > > > Description > ------- > > I noticed that there was some code in src/test/python/twitter that shouldn't've been there. This also uncovered a malformed BUILD and some broken tests. > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/jobs.py 1d0f37ea447d4915479749d3a0e24445984153cd > src/test/python/apache/aurora/client/BUILD 27ebaf5cbda9e80b8beb47c85ad17a85eb234b1b > src/test/python/apache/aurora/client/cli/BUILD bf21590c35f105c0efb85a232fad9d3cd36329c6 > src/test/python/apache/aurora/client/cli/test_status.py 38f301836fa6f95c763e8ffb3e5ce7ac1d139096 > src/test/python/twitter/aurora/client/cli/BUILD e4ea5da77649f9b663eaf999d4602e7e5177f7ad > src/test/python/twitter/aurora/client/cli/test_diff.py 363352f9d607e90f103234aeb086e306ccbdb4ac > > Diff: https://reviews.apache.org/r/16919/diff/ > > > Testing > ------- > > mba=aurora=; PANTS_PYTHON_TEST_FAILSOFT=1 ./pants src/test/python/apache/aurora/client/cli:all -v > Build operating on targets: OrderedSet([PythonTestSuite(src/test/python/apache/aurora/client/cli/BUILD:all)]) > ========================================================== test session starts =========================================================== > platform darwin -- Python 2.7.6 -- pytest-2.5.1 -- /Users/wickman/Python/CPython-2.7.6/bin/python2.7 > collected 17 items > > src/test/python/apache/aurora/client/cli/test_create.py:132: TestClientCreateCommand.test_create_job_delayed PASSED > src/test/python/apache/aurora/client/cli/test_create.py:157: TestClientCreateCommand.test_create_job_failed PASSED > src/test/python/apache/aurora/client/cli/test_create.py:183: TestClientCreateCommand.test_create_job_failed_invalid_config PASSED > src/test/python/apache/aurora/client/cli/test_create.py:99: TestClientCreateCommand.test_simple_successful_create_job PASSED > src/test/python/apache/aurora/client/cli/test_kill.py:20: TestInstancesParser.test_parse_instances PASSED > src/test/python/apache/aurora/client/cli/test_kill.py:25: TestInstancesParser.test_parse_none PASSED > src/test/python/apache/aurora/client/cli/test_kill.py:39: TestClientKillCommand.test_kill_job PASSED > src/test/python/apache/aurora/client/cli/test_kill.py:60: TestClientKillCommand.test_kill_job_with_instances PASSED > src/test/python/apache/aurora/client/cli/test_kill.py:80: TestClientKillCommand.test_kill_job_with_instances_deep_api PASSED > src/test/python/apache/aurora/client/cli/test_status.py:110: TestJobStatus.test_status_wildcard FAILED > src/test/python/apache/aurora/client/cli/test_status.py:130: TestJobStatus.test_status_wildcard_two PASSED > src/test/python/apache/aurora/client/cli/test_status.py:97: TestJobStatus.test_successful_status_deep PASSED > src/test/python/apache/aurora/client/cli/test_status.py:85: TestJobStatus.test_successful_status_shallow PASSED > src/test/python/apache/aurora/client/cli/test_status.py:148: TestJobStatus.test_unsuccessful_status_shallow PASSED > src/test/python/apache/aurora/client/cli/test_diff.py:124: TestDiffCommand.test_diff_invalid_config PASSED > src/test/python/apache/aurora/client/cli/test_diff.py:151: TestDiffCommand.test_diff_server_error FAILED > src/test/python/apache/aurora/client/cli/test_diff.py:93: TestDiffCommand.test_successful_diff FAILED > > ================================================================ FAILURES ================================================================ > ___________________________________________________ TestJobStatus.test_status_wildcard ___________________________________________________ > > self = > > def test_status_wildcard(self): > """Test status using a wildcard. It should first call api.get_jobs, and then do a > getTasksStatus on each job.""" > mock_context = FakeAuroraCommandContext() > mock_api = mock_context.get_api('west') > mock_api.check_status.return_value = self.create_status_response() > mock_api.get_jobs.return_value = self.create_getjobs_response() > with contextlib.nested( > patch('apache.aurora.client.cli.jobs.Job.create_context', return_value=mock_context)): > cmd = AuroraCommandLine() > cmd.execute(['job', 'status', '*']) > > # Wildcard should have expanded to two jobs, so there should be two calls > # to check_status. > assert mock_api.check_status.call_count == 2 > > assert (call(AuroraJobKey('example', 'RoleA', 'test', 'hithere')) in > mock_api.check_status.call_args_list) > E AssertionError: assert call(('example', 'RoleA', 'test', 'hithere')) in [call((u'wickman-loc...mmon.aurora_job_key.AuroraJobKey'>(u'wickman-local', 'bozo', 'test', 'hello'))] > E + where call(('example', 'RoleA', 'test', 'hithere')) = call(('example', 'RoleA', 'test', 'hithere')) > E + where ('example', 'RoleA', 'test', 'hithere') = AuroraJobKey('example', 'RoleA', 'test', 'hithere') > E + and [call((u'wickman-loc...mmon.aurora_job_key.AuroraJobKey'>(u'wickman-local', 'bozo', 'test', 'hello'))] = .call_args_list > E + where = .check_status > > src/test/python/apache/aurora/client/cli/test_status.py:125: AssertionError > ------------------------------------------------------------ Captured stdout ------------------------------------------------------------- > Active tasks (3): > cpus: 2, ram: 2 MB, disk: 2 MB > 1970-11-23 10:58:46 RUNNING: Hi there > packages: cpus: 2, ram: 2 MB, disk: 2 MB > 1970-11-23 10:58:46 RUNNING: Hi there > packages: cpus: 2, ram: 2 MB, disk: 2 MB > 1970-11-23 10:58:46 RUNNING: Hi there > packages:Inactive tasks (0): > Active tasks (3): > cpus: 2, ram: 2 MB, disk: 2 MB > 1970-11-23 10:58:46 RUNNING: Hi there > packages: cpus: 2, ram: 2 MB, disk: 2 MB > 1970-11-23 10:58:46 RUNNING: Hi there > packages: cpus: 2, ram: 2 MB, disk: 2 MB > 1970-11-23 10:58:46 RUNNING: Hi there > packages:Inactive tasks (0): > > _________________________________________________ TestDiffCommand.test_diff_server_error _________________________________________________ > > self = > > def test_diff_server_error(self): > """Test the diff command if the user passes a config with an error in it.""" > mock_options = self.setup_mock_options() > (mock_api, mock_scheduler) = self.create_mock_api() > mock_scheduler.getTasksStatus.return_value = self.create_failed_status_response() > self.setup_populate_job_config(mock_scheduler) > with contextlib.nested( > patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler), > patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS), > patch('twitter.common.app.get_options', return_value=mock_options), > patch('subprocess.call', return_value=0), > patch('json.loads', return_value=Mock())) as ( > mock_scheduler_proxy_class, > mock_clusters, > options, > subprocess_patch, > json_patch): > with temporary_file() as fp: > fp.write(self.get_valid_config()) > fp.flush() > cmd = AuroraCommandLine() > result = cmd.execute(['job', 'diff', 'west/bozo/test/hello', fp.name]) > assert result == EXIT_INVALID_PARAMETER > # In this error case, we should have called the server getTasksStatus; > # but since it fails, we shouldn't call populateJobConfig or subprocess. > mock_scheduler.getTasksStatus.assert_called_with( > TaskQuery(jobName='hello', environment='test', owner=Identity(role='bozo'), > statuses=set([ScheduleStatus.PENDING, ScheduleStatus.STARTING, > ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.ASSIGNED, > > ScheduleStatus.RESTARTING, ScheduleStatus.PREEMPTING]))) > > src/test/python/apache/aurora/client/cli/test_diff.py:180: > _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ > > _mock_self = > args = (TaskQuery(taskIds=None, jobName='hello', environment='test', instanceIds=None,... owner=Identity(role='bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13])),) > kwargs = {}, self = > msg = "Expected call: getTasksStatus(TaskQuery(taskIds=None, jobName='hello', environment='test', instanceIds=None, slaveHos...instanceIds=None, slaveHost=None, owner=Identity(role=u'bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13, 16])))" > > > ??? > E AssertionError: Expected call: getTasksStatus(TaskQuery(taskIds=None, jobName='hello', environment='test', instanceIds=None, slaveHost=None, owner=Identity(role='bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13]))) > E Actual call: getTasksStatus(TaskQuery(taskIds=None, jobName=u'hello', environment=u'test', instanceIds=None, slaveHost=None, owner=Identity(role=u'bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13, 16]))) > > /lib/python2.7/site-packages/mock.py:835: AssertionError > ------------------------------------------------------------ Captured stderr ------------------------------------------------------------- > Error executing command: Could not find job to diff against > __________________________________________________ TestDiffCommand.test_successful_diff __________________________________________________ > > self = > > def test_successful_diff(self): > """Test the diff command.""" > (mock_api, mock_scheduler) = self.setup_mock_api() > with contextlib.nested( > patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler), > patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS), > patch('subprocess.call', return_value=0), > patch('json.loads', return_value=Mock())) as (_, _, subprocess_patch, _): > mock_scheduler.getTasksStatus.return_value = self.create_status_response() > self.setup_populate_job_config(mock_scheduler) > with temporary_file() as fp: > fp.write(self.get_valid_config()) > fp.flush() > cmd = AuroraCommandLine() > cmd.execute(['job', 'diff', 'west/bozo/test/hello', fp.name]) > > # Diff should get the task status, populate a config, and run diff. > mock_scheduler.getTasksStatus.assert_called_with( > TaskQuery(jobName='hello', environment='test', owner=Identity(role='bozo'), > statuses=set([ScheduleStatus.PENDING, ScheduleStatus.STARTING, > ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.ASSIGNED, > > ScheduleStatus.RESTARTING, ScheduleStatus.PREEMPTING]))) > > src/test/python/apache/aurora/client/cli/test_diff.py:114: > _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ > > _mock_self = > args = (TaskQuery(taskIds=None, jobName='hello', environment='test', instanceIds=None,... owner=Identity(role='bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13])),) > kwargs = {}, self = > msg = "Expected call: getTasksStatus(TaskQuery(taskIds=None, jobName='hello', environment='test', instanceIds=None, slaveHos...instanceIds=None, slaveHost=None, owner=Identity(role=u'bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13, 16])))" > > > ??? > E AssertionError: Expected call: getTasksStatus(TaskQuery(taskIds=None, jobName='hello', environment='test', instanceIds=None, slaveHost=None, owner=Identity(role='bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13]))) > E Actual call: getTasksStatus(TaskQuery(taskIds=None, jobName=u'hello', environment=u'test', instanceIds=None, slaveHost=None, owner=Identity(role=u'bozo', user=None), statuses=set([0, 1, 2, 6, 9, 12, 13, 16]))) > > /lib/python2.7/site-packages/mock.py:835: AssertionError > ================================================== 3 failed, 14 passed in 0.73 seconds =================================================== > src.test.python.apache.aurora.client.cli.job ..... FAILURE > mba=aurora=; > > > Thanks, > > Brian Wickman > > --===============5818325462882290082==--