From 6fbda60782f9fa7c027a13d38b0df49f59938398 Mon Sep 17 00:00:00 2001
From: Anjana Vakil <anjanavakil@gmail.com>
Date: Mon, 15 Aug 2016 14:38:36 +0200
Subject: [PATCH] Bug 1292300 - Small readability/typo fixes; r=maja_zf

Assorted small changes to the tests in
`test_marionette_runner.py`, including:

- Declare `return_value`, etc. as argument in call
  to `patch`, instead of on separate lines
- Avoid a multi-line `assert` statement
- Fix a typo in `test_add_test_directory`
- Fix some minor pep8/flake8 style issues

MozReview-Commit-ID: 2EkA41dJNms

--HG--
extra : rebase_source : 5676d25a15fe5da3e6828844fe951229fe1b5d4c
---
 .../harness_unit/test_marionette_runner.py    | 32 ++++++++-----------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py b/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
index d78db65d8738..855947f3dfa5 100644
--- a/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
+++ b/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
@@ -235,9 +235,9 @@ def test_parse_arg_socket_timeout_with_multiple_values(sock_timeout_value):
 
 def test_call_harness_with_no_args_yields_num_failures(runner_class):
     with patch(
-        'marionette.runtests.MarionetteHarness.parse_args'
+        'marionette.runtests.MarionetteHarness.parse_args',
+        return_value={'tests': []}
     ) as parse_args:
-        parse_args.return_value = {'tests': []}
         failed_or_crashed = MarionetteHarness(runner_class).run()
         assert parse_args.call_count == 1
     assert failed_or_crashed == 0
@@ -271,9 +271,9 @@ def test_parsing_testvars(mach_parsed_kwargs):
          "device": {"stuff": "buzz"}
     }
     with patch(
-        'marionette.runtests.MarionetteTestRunner._load_testvars'
+        'marionette.runtests.MarionetteTestRunner._load_testvars',
+        return_value=testvars_json_loads
     ) as load:
-        load.return_value = testvars_json_loads
         runner = MarionetteTestRunner(**mach_parsed_kwargs)
         assert runner.testvars == expected_dict
         assert load.call_count == 1
@@ -285,8 +285,7 @@ def test_load_testvars_throws_expected_errors(mach_parsed_kwargs):
     with pytest.raises(IOError) as io_exc:
         runner._load_testvars()
     assert 'does not exist' in io_exc.value.message
-    with patch('os.path.exists') as exists:
-        exists.return_value = True
+    with patch('os.path.exists', return_value=True):
         with patch('__builtin__.open', mock_open(read_data='[not {valid JSON]')):
             with pytest.raises(Exception) as json_exc:
                 runner._load_testvars()
@@ -308,7 +307,7 @@ def test_crash_is_recorded_as_error(empty_marionette_test,
     assert len(result.errors) == 0
     assert len(result.failures) == 0
     assert result.testsRun == 1
-    assert result.shouldStop == False
+    assert result.shouldStop is False
     result.stopTest(empty_marionette_test)
     assert result.shouldStop == has_crashed
     if has_crashed:
@@ -342,29 +341,26 @@ def test_add_test_module(runner):
     tests = ['test_something.py', 'testSomething.js', 'bad_test.py']
     assert len(runner.tests) == 0
     for test in tests:
-        with patch ('os.path.abspath') as abspath:
-            abspath.return_value = test
+        with patch('os.path.abspath', return_value=test) as abspath:
             runner.add_test(test)
         assert abspath.called
-        assert {'filepath': test,
-                'expected': 'pass',
-                'test_container': None} in runner.tests
+        expected = {'filepath': test, 'expected': 'pass', 'test_container': None}
+        assert expected in runner.tests
     # add_test doesn't validate module names; 'bad_test.py' gets through
     assert len(runner.tests) == 3
 
+
 def test_add_test_directory(runner):
     test_dir = 'path/to/tests'
     dir_contents = [
         (test_dir, ('subdir',), ('test_a.py', 'test_a.js', 'bad_test_a.py', 'bad_test_a.js')),
-        (test_dir + '/subdir', (), ('test_b.py', 'test_b.js', 'bad_test_a.py', 'bad_test_b.js')),
+        (test_dir + '/subdir', (), ('test_b.py', 'test_b.js', 'bad_test_b.py', 'bad_test_b.js')),
     ]
     tests = list(dir_contents[0][2] + dir_contents[1][2])
     assert len(runner.tests) == 0
-    with patch('os.path.isdir') as isdir:
-        # Need to use side effect to make isdir return True for test_dir and False for tests
-        isdir.side_effect = [True] + [False for i in tests]
-        with patch('os.walk') as walk:
-            walk.return_value = dir_contents
+    # Need to use side effect to make isdir return True for test_dir and False for tests
+    with patch('os.path.isdir', side_effect=[True] + [False for t in tests]) as isdir:
+        with patch('os.walk', return_value=dir_contents) as walk:
             runner.add_test(test_dir)
     assert isdir.called and walk.called
     for test in runner.tests: