aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David McLaughlin" <da...@dmclaughlin.com>
Subject Re: Review Request 27058: Add specs to instances of Mock in Python tests.
Date Fri, 24 Oct 2014 20:09:21 GMT

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

(Updated Oct. 24, 2014, 8:09 p.m.)


Review request for Aurora, Mark Chu-Carroll and Zameer Manji.


Changes
-------

rebase.


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 (updated)
-----

  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