Skip to content

Commit

Permalink
Get rid of contextlib.nested() for py3
Browse files Browse the repository at this point in the history
contextlib.nested() is missing completely in Python 3.

Since 2.7, we can use multiple context managers in a 'with' statement,
like so:

    with thing1() as t1, thing2() as t2:
        do_stuff()

Now, if we had some code that needed to nest an arbitrary number of
context managers, there's stuff we could do with contextlib.ExitStack
and such... but we don't. We only use contextlib.nested() in tests to
set up bunches of mocks without crazy-deep indentation, and all that
stuff fits perfectly into multiple-context-manager 'with' statements.

Change-Id: Id472958b007948f05dbd4c7fb8cf3ffab58e2681
  • Loading branch information
smerritt committed Oct 23, 2015
1 parent e0e8d8a commit e31ecb2
Show file tree
Hide file tree
Showing 13 changed files with 325 additions and 400 deletions.
114 changes: 53 additions & 61 deletions test/unit/account/test_reaper.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

from logging import DEBUG
from mock import patch, call, DEFAULT
from contextlib import nested
import six

from swift.account import reaper
Expand Down Expand Up @@ -415,15 +414,14 @@ def test_reap_container_get_object_fail(self):
self.reap_obj_fail = False
self.amount_delete_fail = 0
self.max_delete_fail = 0
ctx = [patch('swift.account.reaper.direct_get_container',
self.fake_direct_get_container),
patch('swift.account.reaper.direct_delete_container',
self.fake_direct_delete_container),
patch('swift.account.reaper.AccountReaper.get_container_ring',
self.fake_container_ring),
patch('swift.account.reaper.AccountReaper.reap_object',
self.fake_reap_object)]
with nested(*ctx):
with patch('swift.account.reaper.direct_get_container',
self.fake_direct_get_container), \
patch('swift.account.reaper.direct_delete_container',
self.fake_direct_delete_container), \
patch('swift.account.reaper.AccountReaper.get_container_ring',
self.fake_container_ring), \
patch('swift.account.reaper.AccountReaper.reap_object',
self.fake_reap_object):
r.reap_container('a', 'partition', acc_nodes, 'c')
self.assertEqual(r.logger.get_increment_counts()['return_codes.4'], 1)
self.assertEqual(r.stats_containers_deleted, 1)
Expand All @@ -434,15 +432,14 @@ def test_reap_container_partial_fail(self):
self.reap_obj_fail = False
self.amount_delete_fail = 0
self.max_delete_fail = 2
ctx = [patch('swift.account.reaper.direct_get_container',
self.fake_direct_get_container),
patch('swift.account.reaper.direct_delete_container',
self.fake_direct_delete_container),
patch('swift.account.reaper.AccountReaper.get_container_ring',
self.fake_container_ring),
patch('swift.account.reaper.AccountReaper.reap_object',
self.fake_reap_object)]
with nested(*ctx):
with patch('swift.account.reaper.direct_get_container',
self.fake_direct_get_container), \
patch('swift.account.reaper.direct_delete_container',
self.fake_direct_delete_container), \
patch('swift.account.reaper.AccountReaper.get_container_ring',
self.fake_container_ring), \
patch('swift.account.reaper.AccountReaper.reap_object',
self.fake_reap_object):
r.reap_container('a', 'partition', acc_nodes, 'c')
self.assertEqual(r.logger.get_increment_counts()['return_codes.4'], 2)
self.assertEqual(r.stats_containers_possibly_remaining, 1)
Expand All @@ -453,15 +450,14 @@ def test_reap_container_full_fail(self):
self.reap_obj_fail = False
self.amount_delete_fail = 0
self.max_delete_fail = 3
ctx = [patch('swift.account.reaper.direct_get_container',
self.fake_direct_get_container),
patch('swift.account.reaper.direct_delete_container',
self.fake_direct_delete_container),
patch('swift.account.reaper.AccountReaper.get_container_ring',
self.fake_container_ring),
patch('swift.account.reaper.AccountReaper.reap_object',
self.fake_reap_object)]
with nested(*ctx):
with patch('swift.account.reaper.direct_get_container',
self.fake_direct_get_container), \
patch('swift.account.reaper.direct_delete_container',
self.fake_direct_delete_container), \
patch('swift.account.reaper.AccountReaper.get_container_ring',
self.fake_container_ring), \
patch('swift.account.reaper.AccountReaper.reap_object',
self.fake_reap_object):
r.reap_container('a', 'partition', acc_nodes, 'c')
self.assertEqual(r.logger.get_increment_counts()['return_codes.4'], 3)
self.assertEqual(r.stats_containers_remaining, 1)
Expand Down Expand Up @@ -532,11 +528,10 @@ def test_reap_account_no_container(self):
self.r = r = self.init_reaper({}, fakelogger=True)
self.called_amount = 0
r.start_time = time.time()
ctx = [patch('swift.account.reaper.AccountReaper.reap_container',
self.fake_reap_container),
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring)]
with nested(*ctx):
with patch('swift.account.reaper.AccountReaper.reap_container',
self.fake_reap_container), \
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring):
nodes = r.get_account_ring().get_part_nodes()
self.assertTrue(r.reap_account(broker, 'partition', nodes))
self.assertTrue(r.logger.get_lines_for_level(
Expand All @@ -548,13 +543,12 @@ def test_reap_device(self):
self.called_amount = 0
conf = {'devices': devices}
r = self.init_reaper(conf)
ctx = [patch('swift.account.reaper.AccountBroker',
FakeAccountBroker),
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring),
patch('swift.account.reaper.AccountReaper.reap_account',
self.fake_reap_account)]
with nested(*ctx):
with patch('swift.account.reaper.AccountBroker',
FakeAccountBroker), \
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring), \
patch('swift.account.reaper.AccountReaper.reap_account',
self.fake_reap_account):
r.reap_device('sda1')
self.assertEqual(self.called_amount, 1)

Expand All @@ -563,13 +557,12 @@ def test_reap_device_with_ts(self):
self.called_amount = 0
conf = {'devices': devices}
r = self.init_reaper(conf=conf)
ctx = [patch('swift.account.reaper.AccountBroker',
FakeAccountBroker),
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring),
patch('swift.account.reaper.AccountReaper.reap_account',
self.fake_reap_account)]
with nested(*ctx):
with patch('swift.account.reaper.AccountBroker',
FakeAccountBroker), \
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring), \
patch('swift.account.reaper.AccountReaper.reap_account',
self.fake_reap_account):
r.reap_device('sda1')
self.assertEqual(self.called_amount, 0)

Expand All @@ -578,13 +571,12 @@ def test_reap_device_with_not_my_ip(self):
self.called_amount = 0
conf = {'devices': devices}
r = self.init_reaper(conf, myips=['10.10.1.2'])
ctx = [patch('swift.account.reaper.AccountBroker',
FakeAccountBroker),
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring),
patch('swift.account.reaper.AccountReaper.reap_account',
self.fake_reap_account)]
with nested(*ctx):
with patch('swift.account.reaper.AccountBroker',
FakeAccountBroker), \
patch('swift.account.reaper.AccountReaper.get_account_ring',
self.fake_account_ring), \
patch('swift.account.reaper.AccountReaper.reap_account',
self.fake_reap_account):
r.reap_device('sda1')
self.assertEqual(self.called_amount, 0)

Expand Down Expand Up @@ -627,14 +619,14 @@ def fake_reap_container(self, account, account_partition,
account_nodes, container):
container_reaped[0] += 1

ctx = [patch('swift.account.reaper.AccountBroker',
FakeAccountBroker),
patch('swift.account.reaper.AccountBroker.list_containers_iter',
fake_list_containers_iter),
patch('swift.account.reaper.AccountReaper.reap_container',
fake_reap_container), ]
fake_ring = FakeRing()
with nested(*ctx):
with patch('swift.account.reaper.AccountBroker',
FakeAccountBroker), \
patch(
'swift.account.reaper.AccountBroker.list_containers_iter',
fake_list_containers_iter), \
patch('swift.account.reaper.AccountReaper.reap_container',
fake_reap_container):
fake_broker = FakeAccountBroker(['c', 'd', 'e'])
r.reap_account(fake_broker, 10, fake_ring.nodes, 0)
self.assertEqual(container_reaped[0], 1)
Expand Down
54 changes: 26 additions & 28 deletions test/unit/cli/test_recon.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from contextlib import nested
import json
import mock
import os
Expand Down Expand Up @@ -240,11 +239,8 @@ def test_get_ringmd5(self):
mock_scout.return_value = scout_instance
stdout = StringIO()
mock_hash = mock.MagicMock()
patches = [
mock.patch('sys.stdout', new=stdout),
mock.patch('swift.cli.recon.md5', new=mock_hash),
]
with nested(*patches):
with mock.patch('sys.stdout', new=stdout), \
mock.patch('swift.cli.recon.md5', new=mock_hash):
mock_hash.return_value.hexdigest.return_value = \
empty_file_hash
self.recon_instance.get_ringmd5(hosts, self.swift_dir)
Expand Down Expand Up @@ -295,11 +291,9 @@ def mock_scout_quarantine(app, host):
return url, response, status, 0, 0

stdout = StringIO()
patches = [
mock.patch('swift.cli.recon.Scout.scout', mock_scout_quarantine),
mock.patch('sys.stdout', new=stdout),
]
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout',
mock_scout_quarantine), \
mock.patch('sys.stdout', new=stdout):
self.recon_instance.quarantine_check(hosts)

output = stdout.getvalue()
Expand Down Expand Up @@ -332,11 +326,9 @@ def mock_scout_driveaudit(app, host):
return url, response, status, 0, 0

stdout = StringIO()
patches = [
mock.patch('swift.cli.recon.Scout.scout', mock_scout_driveaudit),
mock.patch('sys.stdout', new=stdout),
]
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout',
mock_scout_driveaudit), \
mock.patch('sys.stdout', new=stdout):
self.recon_instance.driveaudit_check(hosts)

output = stdout.getvalue()
Expand Down Expand Up @@ -394,19 +386,15 @@ def mock_scout_server_type(app, host):
return url, response, status

stdout = StringIO()
patches = [
mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type),
mock.patch('sys.stdout', new=stdout),
]

res_object = 'Invalid: http://127.0.0.1:6010/ is object-server'
res_container = 'Invalid: http://127.0.0.1:6011/ is container-server'
res_account = 'Invalid: http://127.0.0.1:6012/ is account-server'
valid = "1/1 hosts ok, 0 error[s] while checking hosts."

# Test for object server type - default
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type), \
mock.patch('sys.stdout', new=stdout):
self.recon.server_type_check(hosts)

output = stdout.getvalue()
Expand All @@ -415,15 +403,19 @@ def mock_scout_server_type(app, host):
stdout.truncate(0)

# Test ok for object server type - default
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type), \
mock.patch('sys.stdout', new=stdout):
self.recon.server_type_check([hosts[0]])

output = stdout.getvalue()
self.assertTrue(valid in output.splitlines())
stdout.truncate(0)

# Test for account server type
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type), \
mock.patch('sys.stdout', new=stdout):
self.recon.server_type = 'account'
self.recon.server_type_check(hosts)

Expand All @@ -433,7 +425,9 @@ def mock_scout_server_type(app, host):
stdout.truncate(0)

# Test ok for account server type
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type), \
mock.patch('sys.stdout', new=stdout):
self.recon.server_type = 'account'
self.recon.server_type_check([hosts[2]])

Expand All @@ -442,7 +436,9 @@ def mock_scout_server_type(app, host):
stdout.truncate(0)

# Test for container server type
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type), \
mock.patch('sys.stdout', new=stdout):
self.recon.server_type = 'container'
self.recon.server_type_check(hosts)

Expand All @@ -452,7 +448,9 @@ def mock_scout_server_type(app, host):
stdout.truncate(0)

# Test ok for container server type
with nested(*patches):
with mock.patch('swift.cli.recon.Scout.scout_server_type',
mock_scout_server_type), \
mock.patch('sys.stdout', new=stdout):
self.recon.server_type = 'container'
self.recon.server_type_check([hosts[1]])

Expand Down
10 changes: 4 additions & 6 deletions test/unit/common/middleware/test_dlo.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import contextlib
import hashlib
import json
import mock
Expand Down Expand Up @@ -701,12 +700,11 @@ def mock_is_success(status_int):
'/v1/AUTH_test/mancon/manifest',
environ={'REQUEST_METHOD': 'GET'})

with contextlib.nested(
mock.patch('swift.common.request_helpers.time.time',
mock_time),
with mock.patch('swift.common.request_helpers.time.time',
mock_time), \
mock.patch('swift.common.request_helpers.is_success',
mock_is_success),
mock.patch.object(dlo, 'is_success', mock_is_success)):
mock_is_success), \
mock.patch.object(dlo, 'is_success', mock_is_success):
status, headers, body, exc = self.call_dlo(
req, expect_exception=True)

Expand Down
15 changes: 7 additions & 8 deletions test/unit/common/middleware/test_slo.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import hashlib
import time
import unittest
from contextlib import nested
from mock import patch
from hashlib import md5
from swift.common import swob, utils
Expand Down Expand Up @@ -2250,13 +2249,13 @@ def mock_is_success(status_int):
'/v1/AUTH_test/gettest/manifest-abcd',
environ={'REQUEST_METHOD': 'GET'})

with nested(patch.object(slo, 'is_success', mock_is_success),
patch('swift.common.request_helpers.time.time',
mock_time),
patch('swift.common.request_helpers.is_success',
mock_is_success)):
status, headers, body, exc = self.call_slo(
req, expect_exception=True)
with patch.object(slo, 'is_success', mock_is_success), \
patch('swift.common.request_helpers.time.time',
mock_time), \
patch('swift.common.request_helpers.is_success',
mock_is_success):
status, headers, body, exc = self.call_slo(
req, expect_exception=True)

self.assertIsInstance(exc, SegmentError)
self.assertEqual(status, '200 OK')
Expand Down
6 changes: 3 additions & 3 deletions test/unit/common/middleware/test_tempauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import json
import unittest
from contextlib import contextmanager, nested
from contextlib import contextmanager
from base64 import b64encode
from time import time
import mock
Expand Down Expand Up @@ -273,8 +273,8 @@ def test_auth_with_s3_authorization(self):
headers={'X-Auth-Token': 't',
'AUTHORIZATION': 'AWS s3:s3:pass'})

with nested(mock.patch('base64.urlsafe_b64decode'),
mock.patch('base64.encodestring')) as (msg, sign):
with mock.patch('base64.urlsafe_b64decode') as msg, \
mock.patch('base64.encodestring') as sign:
msg.return_value = ''
sign.return_value = 'pass'
resp = req.get_response(local_auth)
Expand Down
Loading

0 comments on commit e31ecb2

Please sign in to comment.