From c878ec6eeeccb9056dda578d5ae497468d04c211 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 2 Sep 2021 05:00:15 -0700 Subject: [PATCH 1/5] Install nose no-network and timer plugins, use no-network in test & coverage --- requirements-dev.txt | 2 ++ script/coverage | 2 +- script/test | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 522f112..39faa18 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,6 +1,8 @@ coverage mock nose +nose-no-network +nose-timer pycodestyle==2.6.0 pyflakes==2.2.0 readme_renderer[md]==26.0 diff --git a/script/coverage b/script/coverage index db8e219..2ca8d4b 100755 --- a/script/coverage +++ b/script/coverage @@ -36,7 +36,7 @@ grep -r -I --line-number "# pragma: +no.*cover" octodns && { exit 1 } -coverage run --branch --source=octodns --omit=octodns/cmds/* "$(command -v nosetests)" --with-xunit "$@" +coverage run --branch --source=octodns --omit=octodns/cmds/* "$(command -v nosetests)" --with-no-network --with-xunit "$@" coverage html coverage xml coverage report --show-missing diff --git a/script/test b/script/test index 98bae20..b074595 100755 --- a/script/test +++ b/script/test @@ -30,4 +30,4 @@ export ARM_CLIENT_SECRET= export ARM_TENANT_ID= export ARM_SUBSCRIPTION_ID= -nosetests "$@" +nosetests --with-no-network "$@" From 2efb55068667ca768f847d6597e7b89ed70b75e0 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 2 Sep 2021 10:03:39 -0700 Subject: [PATCH 2/5] Rework TransipProvider to support mocking and avoid network calls during __init__ --- octodns/provider/transip.py | 32 ++++++++++++++++++-------- tests/test_octodns_provider_transip.py | 13 +++++++---- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/octodns/provider/transip.py b/octodns/provider/transip.py index 2bdedc4..67011f9 100644 --- a/octodns/provider/transip.py +++ b/octodns/provider/transip.py @@ -57,26 +57,38 @@ class TransipProvider(BaseProvider): TIMEOUT = 15 ROOT_RECORD = '@' - def __init__(self, id, account, key=None, key_file=None, *args, **kwargs): + def __init__(self, id, account, key=None, key_file=None, *args, **kwargs): self.log = getLogger('TransipProvider[{}]'.format(id)) self.log.debug('__init__: id=%s, account=%s, token=***', id, account) super(TransipProvider, self).__init__(id, *args, **kwargs) - if key_file is not None: - self._client = DomainService(account, private_key_file=key_file) - elif key is not None: - self._client = DomainService(account, private_key=key) - else: + if key is None and key_file is None: raise TransipConfigException( - 'Missing `key` of `key_file` parameter in config' + 'Missing `key` or `key_file` parameter in config' ) self.account = account self.key = key + self.key_file = key_file + self._client = None self._currentZone = {} + @property + def client(self): + # This can't happen in __init__ b/c it makes network calls during the + # construction of the object and that before the tests have had a + # chance to install the mock client + if self._client is None: + if self.key_file is not None: + self._client = DomainService(self.account, + private_key_file=self.key_file) + else: # we checked key in __init__ so can assume it's not None + self._client = DomainService(self.account, + private_key=self.key) + return self._client + def populate(self, zone, target=False, lenient=False): exists = False @@ -86,7 +98,7 @@ class TransipProvider(BaseProvider): before = len(zone.records) try: - zoneInfo = self._client.get_info(zone.name[:-1]) + zoneInfo = self.client.get_info(zone.name[:-1]) except WebFault as e: if e.fault.faultcode == '102' and target is False: # Zone not found in account, and not a target so just @@ -136,7 +148,7 @@ class TransipProvider(BaseProvider): self._currentZone = plan.desired try: - self._client.get_info(plan.desired.name[:-1]) + self.client.get_info(plan.desired.name[:-1]) except WebFault as e: self.log.exception('_apply: get_info failed') raise e @@ -155,7 +167,7 @@ class TransipProvider(BaseProvider): _dns_entries.extend(entries_for(name, record)) try: - self._client.set_dns_entries(plan.desired.name[:-1], _dns_entries) + self.client.set_dns_entries(plan.desired.name[:-1], _dns_entries) except WebFault as e: self.log.warning(('_apply: Set DNS returned ' + 'one or more errors: {}').format( diff --git a/tests/test_octodns_provider_transip.py b/tests/test_octodns_provider_transip.py index 234c95e..4817f7d 100644 --- a/tests/test_octodns_provider_transip.py +++ b/tests/test_octodns_provider_transip.py @@ -15,7 +15,6 @@ from unittest import TestCase from octodns.provider.transip import TransipProvider from octodns.provider.yaml import YamlProvider from octodns.zone import Zone -from transip.service.domain import DomainService from transip.service.objects import DnsEntry @@ -32,12 +31,11 @@ class MockResponse(object): dnsEntries = [] -class MockDomainService(DomainService): +class MockDomainService(object): def __init__(self, *args, **kwargs): - super(MockDomainService, self).__init__('MockDomainService', *args, - **kwargs) self.mockupEntries = [] + self.throw_auth_fault = False def mockup(self, records): @@ -67,6 +65,9 @@ class MockDomainService(DomainService): # Skips authentication layer and returns the entries loaded by "Mockup" def get_info(self, domain_name): + if self.throw_auth_fault: + self.raiseInvalidAuth() + # Special 'domain' to trigger error if str(domain_name) == str('notfound.unit.tests'): self.raiseZoneNotFound() @@ -140,7 +141,7 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd TransipProvider('test', 'unittest') self.assertEquals( - str('Missing `key` of `key_file` parameter in config'), + str('Missing `key` or `key_file` parameter in config'), str(ctx.exception)) TransipProvider('test', 'unittest', key=self.bogus_key) @@ -155,6 +156,8 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd # Live test against API, will fail in an unauthorized error with self.assertRaises(WebFault) as ctx: provider = TransipProvider('test', 'unittest', self.bogus_key) + provider._client = MockDomainService('unittest', self.bogus_key) + provider._client.throw_auth_fault = True zone = Zone('unit.tests.', []) provider.populate(zone, True) From 4f036c77f8a3ff930a33cf02dab62bc33a1e7504 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 2 Sep 2021 10:48:40 -0700 Subject: [PATCH 3/5] Mock boto's unstubbed metadata api access --- tests/test_octodns_provider_route53.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index b3e5ba4..a80122e 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -394,8 +394,12 @@ class TestRoute53Provider(TestCase): return (provider, stubber) - def test_process_desired_zone(self): + # with fallback boto makes an unstubbed call to the 169. metadata api, this + # stubs that bit out + @patch('botocore.credentials.CredentialResolver.load_credentials') + def test_process_desired_zone(self, fetch_metadata_token_mock): provider, stubber = self._get_stubbed_fallback_auth_provider() + fetch_metadata_token_mock.side_effect = [None] # No records, essentially a no-op desired = Zone('unit.tests.', []) @@ -527,8 +531,12 @@ class TestRoute53Provider(TestCase): list(got.records)[0].dynamic.rules[0].data['geos']) self.assertFalse('geos' in list(got.records)[0].dynamic.rules[1].data) - def test_populate_with_fallback(self): + # with fallback boto makes an unstubbed call to the 169. metadata api, this + # stubs that bit out + @patch('botocore.credentials.CredentialResolver.load_credentials') + def test_populate_with_fallback(self, fetch_metadata_token_mock): provider, stubber = self._get_stubbed_fallback_auth_provider() + fetch_metadata_token_mock.side_effect = [None] got = Zone('unit.tests.', []) with self.assertRaises(ClientError): From d374dc355bf7b788d975979fa00a87c716fe63f3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 2 Sep 2021 13:00:38 -0700 Subject: [PATCH 4/5] Use patch to mock in transip tests, rework bits to make more mockable --- octodns/provider/transip.py | 34 ++++++++-------------- tests/test_octodns_provider_transip.py | 39 +++++++++++++++++--------- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/octodns/provider/transip.py b/octodns/provider/transip.py index 67011f9..d3e2018 100644 --- a/octodns/provider/transip.py +++ b/octodns/provider/transip.py @@ -63,31 +63,21 @@ class TransipProvider(BaseProvider): account) super(TransipProvider, self).__init__(id, *args, **kwargs) - if key is None and key_file is None: + if key_file is not None: + self._client = self._domain_service(account, + private_key_file=key_file) + elif key is not None: + self._client = self._domain_service(account, private_key=key) + else: raise TransipConfigException( 'Missing `key` or `key_file` parameter in config' ) - self.account = account - self.key = key - self.key_file = key_file - - self._client = None self._currentZone = {} - @property - def client(self): - # This can't happen in __init__ b/c it makes network calls during the - # construction of the object and that before the tests have had a - # chance to install the mock client - if self._client is None: - if self.key_file is not None: - self._client = DomainService(self.account, - private_key_file=self.key_file) - else: # we checked key in __init__ so can assume it's not None - self._client = DomainService(self.account, - private_key=self.key) - return self._client + def _domain_service(self, *args, **kwargs): + 'This exists only for mocking purposes' + return DomainService(*args, **kwargs) def populate(self, zone, target=False, lenient=False): @@ -98,7 +88,7 @@ class TransipProvider(BaseProvider): before = len(zone.records) try: - zoneInfo = self.client.get_info(zone.name[:-1]) + zoneInfo = self._client.get_info(zone.name[:-1]) except WebFault as e: if e.fault.faultcode == '102' and target is False: # Zone not found in account, and not a target so just @@ -148,7 +138,7 @@ class TransipProvider(BaseProvider): self._currentZone = plan.desired try: - self.client.get_info(plan.desired.name[:-1]) + self._client.get_info(plan.desired.name[:-1]) except WebFault as e: self.log.exception('_apply: get_info failed') raise e @@ -167,7 +157,7 @@ class TransipProvider(BaseProvider): _dns_entries.extend(entries_for(name, record)) try: - self.client.set_dns_entries(plan.desired.name[:-1], _dns_entries) + self._client.set_dns_entries(plan.desired.name[:-1], _dns_entries) except WebFault as e: self.log.warning(('_apply: Set DNS returned ' + 'one or more errors: {}').format( diff --git a/tests/test_octodns_provider_transip.py b/tests/test_octodns_provider_transip.py index 4817f7d..8a2e11a 100644 --- a/tests/test_octodns_provider_transip.py +++ b/tests/test_octodns_provider_transip.py @@ -10,6 +10,7 @@ from six import text_type from suds import WebFault +from mock import patch from unittest import TestCase from octodns.provider.transip import TransipProvider @@ -136,7 +137,11 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd source.populate(expected) return expected - def test_init(self): + @patch('octodns.provider.transip.TransipProvider._domain_service', + return_value=MockDomainService()) + def test_init(self, _): + + # No key nor key_file with self.assertRaises(Exception) as ctx: TransipProvider('test', 'unittest') @@ -144,19 +149,26 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd str('Missing `key` or `key_file` parameter in config'), str(ctx.exception)) + # With key TransipProvider('test', 'unittest', key=self.bogus_key) - # Existence and content of the key is tested in the SDK on client call + # With key_file TransipProvider('test', 'unittest', key_file='/fake/path') - def test_populate(self): + @patch('suds.client.Client.__init__', new=lambda *args, **kwargs: None) + def test_domain_service(self): + # Special case smoke test for DomainService to get coverage + TransipProvider('test', 'unittest', key=self.bogus_key) + + @patch('octodns.provider.transip.TransipProvider._domain_service', + return_value=MockDomainService()) + def test_populate(self, _): _expected = self.make_expected() # Unhappy Plan - Not authenticated # Live test against API, will fail in an unauthorized error with self.assertRaises(WebFault) as ctx: provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) provider._client.throw_auth_fault = True zone = Zone('unit.tests.', []) provider.populate(zone, True) @@ -166,12 +178,14 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd self.assertEquals(str('200'), ctx.exception.fault.faultcode) + # No more auth problems + provider._client.throw_auth_fault = False + # Unhappy Plan - Zone does not exists # Will trigger an exception if provider is used as a target for a # non-existing zone with self.assertRaises(Exception) as ctx: provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) zone = Zone('notfound.unit.tests.', []) provider.populate(zone, True) @@ -187,13 +201,11 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd # Won't trigger an exception if provider is NOT used as a target for a # non-existing zone. provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) zone = Zone('notfound.unit.tests.', []) provider.populate(zone, False) # Happy Plan - Populate with mockup records provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) provider._client.mockup(_expected.records) zone = Zone('unit.tests.', []) provider.populate(zone, False) @@ -211,19 +223,19 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd # Happy Plan - Even if the zone has no records the zone should exist provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) zone = Zone('unit.tests.', []) exists = provider.populate(zone, True) self.assertTrue(exists, 'populate should return true') return - def test_plan(self): + @patch('octodns.provider.transip.TransipProvider._domain_service', + return_value=MockDomainService()) + def test_plan(self, _): _expected = self.make_expected() # Test Happy plan, only create provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) plan = provider.plan(_expected) self.assertEqual(15, plan.change_counts['Create']) @@ -232,12 +244,13 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd return - def test_apply(self): + @patch('octodns.provider.transip.TransipProvider._domain_service', + return_value=MockDomainService()) + def test_apply(self, _): _expected = self.make_expected() # Test happy flow. Create all supoorted records provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) plan = provider.plan(_expected) self.assertEqual(15, len(plan.changes)) changes = provider.apply(plan) @@ -249,7 +262,6 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd changes = [] # reset changes with self.assertRaises(Exception) as ctx: provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) plan = provider.plan(_expected) plan.desired.name = 'notfound.unit.tests.' changes = provider.apply(plan) @@ -268,7 +280,6 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd with self.assertRaises(Exception) as ctx: provider = TransipProvider('test', 'unittest', self.bogus_key) - provider._client = MockDomainService('unittest', self.bogus_key) plan = provider.plan(_expected) plan.desired.name = 'failsetdns.unit.tests.' changes = provider.apply(plan) From 6354e45d7cf9f7cadea8351e0dd3a66dbe31c612 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 2 Sep 2021 13:21:28 -0700 Subject: [PATCH 5/5] Configurable domain_create_sleep in EasyDnsProvider, 0s for tests --- octodns/provider/easydns.py | 20 ++++++++++---------- requirements-dev.txt | 6 ++++++ tests/test_octodns_provider_easydns.py | 6 ++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/octodns/provider/easydns.py b/octodns/provider/easydns.py index 67c846a..2ea0c4b 100644 --- a/octodns/provider/easydns.py +++ b/octodns/provider/easydns.py @@ -48,19 +48,19 @@ class EasyDNSClient(object): # Domain Portfolio domain_portfolio = 'myport' - def __init__(self, token, api_key, currency, portfolio, sandbox): + def __init__(self, token, api_key, currency, portfolio, sandbox, + domain_create_sleep): self.log = logging.getLogger('EasyDNSProvider[{}]'.format(id)) - self.token = token - self.api_key = api_key self.default_currency = currency self.domain_portfolio = portfolio - self.apienv = 'sandbox' if sandbox else 'live' - auth_key = '{}:{}'.format(self.token, self.api_key) - self.auth_key = base64.b64encode(auth_key.encode("utf-8")) + self.domain_create_sleep = domain_create_sleep + + auth_key = '{}:{}'.format(token, api_key) + auth_key = base64.b64encode(auth_key.encode("utf-8")) self.base_path = self.SANDBOX if sandbox else self.LIVE sess = Session() sess.headers.update({'Authorization': 'Basic {}' - .format(self.auth_key.decode('utf-8'))}) + .format(auth_key.decode('utf-8'))}) sess.headers.update({'accept': 'application/json'}) self._sess = sess @@ -99,7 +99,7 @@ class EasyDNSClient(object): # we need to delete those default record so we can sync with the source # records, first we'll sleep for a second before gathering new records # We also create default NS records, but they won't be deleted - sleep(1) + sleep(self.domain_create_sleep) records = self.records(name, True) for record in records: if record['host'] in ('', 'www') \ @@ -163,12 +163,12 @@ class EasyDNSProvider(BaseProvider): 'SRV', 'NAPTR')) def __init__(self, id, token, api_key, currency='CAD', portfolio='myport', - sandbox=False, *args, **kwargs): + sandbox=False, domain_create_sleep=1, *args, **kwargs): self.log = logging.getLogger('EasyDNSProvider[{}]'.format(id)) self.log.debug('__init__: id=%s, token=***', id) super(EasyDNSProvider, self).__init__(id, *args, **kwargs) self._client = EasyDNSClient(token, api_key, currency, portfolio, - sandbox) + sandbox, domain_create_sleep) self._zone_records = {} def _data_for_multiple(self, _type, records): diff --git a/requirements-dev.txt b/requirements-dev.txt index 39faa18..27b2967 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -8,3 +8,9 @@ pyflakes==2.2.0 readme_renderer[md]==26.0 requests_mock twine==3.2.0; python_version >= '3.2' + +# Profiling tests... +# nose-cprof +# snakeviz +# ./script/test --with-cprof --cprofile-stats-erase +# snakeviz stats.dat diff --git a/tests/test_octodns_provider_easydns.py b/tests/test_octodns_provider_easydns.py index 85492eb..9b50024 100644 --- a/tests/test_octodns_provider_easydns.py +++ b/tests/test_octodns_provider_easydns.py @@ -107,7 +107,8 @@ class TestEasyDNSProvider(TestCase): self.assertEquals('Not Found', text_type(ctx.exception)) def test_apply_not_found(self): - provider = EasyDNSProvider('test', 'token', 'apikey') + provider = EasyDNSProvider('test', 'token', 'apikey', + domain_create_sleep=0) wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, 'test1', { @@ -143,7 +144,8 @@ class TestEasyDNSProvider(TestCase): self.assertEquals('Not Found', text_type(ctx.exception)) def test_domain_create(self): - provider = EasyDNSProvider('test', 'token', 'apikey') + provider = EasyDNSProvider('test', 'token', 'apikey', + domain_create_sleep=0) domain_after_creation = { "tm": 1000000000, "data": [{