aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sweeney" <kevi...@apache.org>
Subject Re: Review Request 27058: Add specs to instances of Mock in Python tests.
Date Fri, 24 Oct 2014 22:49:14 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27058/#review58420
-----------------------------------------------------------

Ship it!


Long live ReviewBot!

- Kevin Sweeney


On Oct. 24, 2014, 3:48 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27058/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 3:48 p.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Zameer Manji.
> 
> 
> Bugs: AURORA-248
>     https://issues.apache.org/jira/browse/AURORA-248
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Use of Mock() without a specification considered harmful. I went through and updated
as many mocks as I could. 
> 
> Any remaining can be classified as:
> 
> 1) Mocks of classes that cannot be spec'd. Almost all instances of SchedulerProxy, which
uses __getattr__ to delegate to the read or write client. 
> 2) Primitives like strings and callback functions or data objects like dicts and pystachio
structs.
> 3) Weird mocks that broke code where they really shouldn't have (off the top of my head
- in test_diff.py and commands/test_run.py) - both when they were spec'd and when they were
replaced with real thrift structs. 
> 
> 
> The remaining offenders:
> 
> $ grep -r --include="*.py" "Mock()" src/test/python 
> src/test/python/apache/aurora/admin/test_host_maintenance.py:    mock_callback = mock.Mock()
> src/test/python/apache/aurora/admin/test_host_maintenance.py:    mock_callback = mock.Mock()
> src/test/python/apache/aurora/client/api/test_api.py:    mock_proxy = Mock()
> src/test/python/apache/aurora/client/api/test_api.py:    mock_get = Mock()
> src/test/python/apache/aurora/client/api/test_api.py:    mock_task_config = Mock()
> src/test/python/apache/aurora/client/api/test_job_monitor.py:    self._scheduler = Mock()
> src/test/python/apache/aurora/client/api/test_quota_check.py:    self._scheduler = Mock()
> src/test/python/apache/aurora/client/api/test_scheduler_client.py:    client._connect_scheduler
= mock.MagicMock()
> src/test/python/apache/aurora/client/api/test_sla.py:    self._scheduler = Mock()
> src/test/python/apache/aurora/client/api/test_task_util.py:    scheduler = Mock()
> src/test/python/apache/aurora/client/cli/test_diff.py:      job = Mock()
> src/test/python/apache/aurora/client/cli/test_diff.py:      job.assignedTask.task.executorConfig.data
= Mock()
> src/test/python/apache/aurora/client/cli/test_diff.py:        patch('json.loads', return_value=Mock()))
as (_, _, subprocess_patch, _):
> src/test/python/apache/aurora/client/cli/test_diff.py:        patch('json.loads', return_value=Mock()))
as (
> src/test/python/apache/aurora/client/cli/test_diff.py:        patch('json.loads', return_value=Mock()))
as (
> src/test/python/apache/aurora/client/cli/test_inspect.py:    raw_config = Mock()
> src/test/python/apache/aurora/client/cli/test_inspect.py:    mock_task = Mock()
> src/test/python/apache/aurora/client/cli/test_inspect.py:    mock_process = Mock()
> src/test/python/apache/aurora/client/cli/test_kill.py:    mock_scheduler_proxy = Mock()
> src/test/python/apache/aurora/client/cli/test_kill.py:    mock_scheduler_proxy = Mock()
> src/test/python/apache/aurora/client/cli/test_kill.py:    mock_scheduler_proxy = Mock()
> src/test/python/apache/aurora/client/cli/test_kill.py:    mock_scheduler_proxy = Mock()
> src/test/python/apache/aurora/client/cli/util.py:    mock_scheduler_proxy = Mock()
> src/test/python/apache/aurora/client/cli/util.py:    mock_scheduler = Mock()
> src/test/python/apache/aurora/client/cli/util.py:    mock_scheduler_client = Mock()
> src/test/python/apache/aurora/client/cli/util.py:    mock_api_factory = Mock()
> src/test/python/apache/aurora/client/commands/test_diff.py:      job = Mock()
> src/test/python/apache/aurora/client/commands/test_diff.py:      job.assignedTask.task.executorConfig.data
= Mock()
> src/test/python/apache/aurora/client/commands/test_diff.py:        patch('json.loads',
return_value=Mock())) as (
> src/test/python/apache/aurora/client/commands/test_diff.py:        patch('json.loads',
return_value=Mock())) as (
> src/test/python/apache/aurora/client/commands/test_diff.py:        patch('json.loads',
return_value=Mock())) as (
> src/test/python/apache/aurora/client/commands/test_listjobs.py:    mock_options = Mock()
> src/test/python/apache/aurora/client/commands/test_listjobs.py:      job = Mock()
> src/test/python/apache/aurora/client/commands/test_maintenance.py:    mock_callback =
Mock()
> src/test/python/apache/aurora/client/commands/test_maintenance.py:      mock_wait = Mock()
> src/test/python/apache/aurora/client/commands/util.py:    mock_scheduler_proxy = Mock()
> src/test/python/apache/aurora/client/commands/util.py:    mock_api_factory = Mock()
> 
> 
> Diffs
> -----
> 
>   src/test/python/apache/aurora/admin/test_admin_util.py f5c8c69c1109d15ee3886fb863014c3285240db1

>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 60c75300501c36ac20a97f78ff18b3ca7af30699

>   src/test/python/apache/aurora/client/cli/test_cron.py c7b71c29d44150162fec8066947623fa91815424

>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2

>   src/test/python/apache/aurora/client/cli/test_sla.py a1a3d8161ba747aa23a5e614e9ae31473d2058c1

>   src/test/python/apache/aurora/client/cli/test_task_run.py 12163df0d2e1e42f2a321603ec10ff9359848216

>   src/test/python/apache/aurora/client/cli/util.py 796c4f9880a0f834a6950472892981e8a6789a97

>   src/test/python/apache/aurora/client/commands/test_cancel_update.py 13aa1fef1d94d46f2837f500606028baa694fa6e

>   src/test/python/apache/aurora/client/commands/test_create.py 4a753fb5942555854538047eb947e5465cdff607

>   src/test/python/apache/aurora/client/commands/test_diff.py 9f1d459e51c663b9ad62bbbbb16a8127568662d1

>   src/test/python/apache/aurora/client/commands/test_hooks.py d4d8d3cd15704353d958e1ef6b220eaa37696a4d

>   src/test/python/apache/aurora/client/commands/test_kill.py 1e13b926379147295a3a1b3d6ce79a727719dedb

>   src/test/python/apache/aurora/client/commands/test_maintenance.py 13d753f6870c9f552903b077e3c38d306ead5bc4

>   src/test/python/apache/aurora/client/commands/test_restart.py efa0849c1f11d9304e2da981dfb9c1d0ff59a15d

>   src/test/python/apache/aurora/client/commands/test_run.py 0c395f7a8106acf3d45842a6f536dfb74b71a309

>   src/test/python/apache/aurora/client/commands/test_ssh.py cf9f425b3dd64afe9d8fcfd70495a3c58108824f

>   src/test/python/apache/aurora/client/commands/test_status.py 9eb8def26692cf5fbd0c20bc96975125e411f0ba

>   src/test/python/apache/aurora/client/commands/test_update.py 555ea0d2727fca61256faf7815945320fcbde55d

> 
> Diff: https://reviews.apache.org/r/27058/diff/
> 
> 
> Testing
> -------
> 
> $ ./pants src/test/python/apache/aurora/:all
> $ build-support/python/checkstyle-check src/test/
> $ build-support/python/isort-check
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message