From c3b17662204a8b2474eebc4070a91496ae57c364 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 6 Jan 2020 16:07:06 +0000 Subject: [PATCH 1/3] bump test requirements --- requirements.txt | 4 ++-- requirements_for_test.txt | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements.txt b/requirements.txt index 9cdd254e6..f6d903e30 100644 --- a/requirements.txt +++ b/requirements.txt @@ -40,12 +40,12 @@ alembic==1.3.2 amqp==1.4.9 anyjson==0.3.3 attrs==19.3.0 -awscli==1.16.305 +awscli==1.16.310 bcrypt==3.1.7 billiard==3.3.0.23 bleach==3.1.0 boto3==1.10.38 -botocore==1.13.41 +botocore==1.13.46 certifi==2019.11.28 chardet==3.0.4 Click==7.0 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 7e9bc35a1..f8edc4dc1 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,11 +1,11 @@ -r requirements.txt flake8==3.7.9 moto==1.3.14 -pytest==5.2.4 +pytest==5.3.2 pytest-env==0.6.2 -pytest-mock==1.11.2 +pytest-mock==2.0.0 pytest-cov==2.8.1 -pytest-xdist==1.30.0 +pytest-xdist==1.31.0 freezegun==0.3.12 requests-mock==1.7.0 # optional requirements for jsonschema From d244146638f70f53d5c96368f7abf2847b2ccde7 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 6 Jan 2020 16:08:33 +0000 Subject: [PATCH 2/3] fix weird test isolation errors with os.environ and boto3 test_config manipulates os.environ. os.environ is an `environ` object, which acts like a dict but isn't in some subtle unknowable ways. The `reload_config` fixture would create a dict copy of the env, and then just call `os.environ = old_env` afterwards. Boto3 would then complain that it couldn't load credentials (despite us using the mock_s3 fixture and also not having creds in the environment in the first place). Not entirely sure why this happens, but it does. For some reason, it being a `dict` instead of an `environ` object causes the mocking of boto3 to fail. The solution is to not overwrite os.environ entirely, rather, use the standard dictionary setitem syntax to update the values to their previous values. Use `clear` to empty the environment too. --- tests/app/test_config.py | 4 +++- tests/conftest.py | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/app/test_config.py b/tests/app/test_config.py index 7cde54486..09a3aae7f 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -21,7 +21,9 @@ def reload_config(): yield - os.environ = old_env + for k, v in old_env.items(): + os.environ[k] = v + importlib.reload(config) diff --git a/tests/conftest.py b/tests/conftest.py index 914200001..9892d9f29 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -136,9 +136,10 @@ def os_environ(): assert type(value) == str super().__setitem__(key, value) - os.environ = EnvironDict() + os.environ.clear() yield - os.environ = old_env + for k, v in old_env.items(): + os.environ[k] = v def pytest_generate_tests(metafunc): From eb72a0279f06d889021faa09703096a5995711f0 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 6 Jan 2020 17:07:06 +0000 Subject: [PATCH 3/3] remove monkeypatch it's interacting strangely with our os.environ stuff and not restoring nicely, probably due to fixture order which is hard to visualise and control. --- tests/app/test_cloudfoundry_config.py | 12 +++++------- tests/app/test_config.py | 12 ++++++------ tests/conftest.py | 10 ++++------ 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index ccf238d49..8e1e5b158 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -26,13 +26,12 @@ def cloudfoundry_config(postgres_config): @pytest.fixture -def cloudfoundry_environ(monkeypatch, cloudfoundry_config): - monkeypatch.setenv('VCAP_SERVICES', json.dumps(cloudfoundry_config)) - monkeypatch.setenv('VCAP_APPLICATION', '{"space_name": "🚀🌌"}') +def cloudfoundry_environ(os_environ, cloudfoundry_config): + os.environ['VCAP_SERVICES'] = json.dumps(cloudfoundry_config) + os.environ['VCAP_APPLICATION'] = '{"space_name": "🚀🌌"}' -@pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') -def test_extract_cloudfoundry_config_populates_other_vars(): +def test_extract_cloudfoundry_config_populates_other_vars(cloudfoundry_environ): extract_cloudfoundry_config() assert os.environ['SQLALCHEMY_DATABASE_URI'] == 'postgres uri' @@ -40,8 +39,7 @@ def test_extract_cloudfoundry_config_populates_other_vars(): assert os.environ['NOTIFY_LOG_PATH'] == '/home/vcap/logs/app.log' -@pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') -def test_set_config_env_vars_ignores_unknown_configs(cloudfoundry_config): +def test_set_config_env_vars_ignores_unknown_configs(cloudfoundry_config, cloudfoundry_environ): cloudfoundry_config['foo'] = {'credentials': {'foo': 'foo'}} cloudfoundry_config['user-provided'].append({ 'name': 'bar', 'credentials': {'bar': 'bar'} diff --git a/tests/app/test_config.py b/tests/app/test_config.py index 09a3aae7f..53c8aacea 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -21,16 +21,17 @@ def reload_config(): yield + os.environ.clear() for k, v in old_env.items(): os.environ[k] = v importlib.reload(config) -def test_load_cloudfoundry_config_if_available(monkeypatch, reload_config): +def test_load_cloudfoundry_config_if_available(reload_config): os.environ['ADMIN_BASE_URL'] = 'env' - monkeypatch.setenv('VCAP_SERVICES', 'some json blob') - monkeypatch.setenv('VCAP_APPLICATION', 'some json blob') + os.environ['VCAP_SERVICES'] = 'some json blob' + os.environ['VCAP_APPLICATION'] = 'some json blob' with mock.patch('app.cloudfoundry_config.extract_cloudfoundry_config', side_effect=cf_conf) as cf_config: # reload config so that its module level code (ie: all of it) is re-instantiated @@ -42,10 +43,9 @@ def test_load_cloudfoundry_config_if_available(monkeypatch, reload_config): assert config.Config.ADMIN_BASE_URL == 'cf' -def test_load_config_if_cloudfoundry_not_available(monkeypatch, reload_config): +def test_load_config_if_cloudfoundry_not_available(reload_config): os.environ['ADMIN_BASE_URL'] = 'env' - - monkeypatch.delenv('VCAP_SERVICES', raising=False) + os.environ.pop('VCAP_SERVICES', None) with mock.patch('app.cloudfoundry_config.extract_cloudfoundry_config') as cf_config: # reload config so that its module level code (ie: all of it) is re-instantiated diff --git a/tests/conftest.py b/tests/conftest.py index 9892d9f29..b0a954ca8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -130,14 +130,12 @@ def os_environ(): """ # for use whenever you expect code to edit environment variables old_env = os.environ.copy() - - class EnvironDict(dict): - def __setitem__(self, key, value): - assert type(value) == str - super().__setitem__(key, value) - os.environ.clear() + yield + + # clear afterwards in case anything extra was added to the environment during the test + os.environ.clear() for k, v in old_env.items(): os.environ[k] = v