Repository: scoutapp/scout_apm_python
Pull request #209 (2019-07-06 10:47:49)

Remove assertion in config
`assert` shouldn't be used in production code since Python's (rarely used) optimize flag (`python -O`) strips them out. Also get the code coverage of this module to 100%.


Patch:
diff --git a/src/scout_apm/core/config.py b/src/scout_apm/core/config.py
index 13acf41..f45591c 100644
--- a/src/scout_apm/core/config.py
+++ b/src/scout_apm/core/config.py
@@ -42,9 +42,10 @@ class ScoutConfig(object):
         for layer in self.layers:
             if layer.has_config(key):
                 return layer
-        else:  # pragma: no cover
-            # Not reachable because ScoutConfigNull returns None for all keys.
-            assert False, "key not found in any layer"
+
+        # Should be unreachable because ScoutConfigNull returns None for all
+        # keys.
+        raise ValueError("key {!r} not found in any layer".format(key))
 
     def log(self):
         logger.debug("Configuration Loaded:")


Test patch:
diff --git a/tests/integration/django_app.py b/tests/integration/django_app.py
index 1c67988..a6bac45 100644
--- a/tests/integration/django_app.py
+++ b/tests/integration/django_app.py
@@ -13,8 +13,8 @@ config = {
     "DATABASES": {
         "default": {"ENGINE": "django.db.backends.sqlite3", "NAME": ":memory:"}
     },
-    "DEBUG": True,
     # Enable the following for debugging exceptions:
+    # "DEBUG": True,
     # "DEBUG_PROPAGATE_EXCEPTIONS": True,
     "ROOT_URLCONF": __name__,
     "SECRET_KEY": "********",
diff --git a/tests/integration/test_django.py b/tests/integration/test_django.py
index 74a7d43..b37fcbd 100644
--- a/tests/integration/test_django.py
+++ b/tests/integration/test_django.py
@@ -142,19 +142,29 @@ def test_server_error(tracked_requests):
     tracked_request = tracked_requests[0]
     assert tracked_request.tags["error"] == "true"
     spans = tracked_requests[0].complete_spans
-    assert [s.operation for s in spans] == [
-        "Template/Compile/<Unknown Template>",
-        "Template/Render/<Unknown Template>",
-        "Controller/tests.integration.django_app.crash",
-        "Middleware",
-    ]
+    operations = [s.operation for s in spans]
+    if django.VERSION >= (1, 9):
+        # Changed in Django 1.9 or later (we only test 1.8 and 1.11 at time of
+        # writing)
+        expected_operations = [
+            "Template/Compile/<Unknown Template>",
+            "Template/Render/<Unknown Template>",
+            "Controller/tests.integration.django_app.crash",
+            "Middleware",
+        ]
+    else:
+        expected_operations = [
+            "Controller/tests.integration.django_app.crash",
+            "Middleware",
+        ]
+    assert operations == expected_operations
 
 
 def test_sql(tracked_requests):
     with app_with_scout() as app:
         response = TestApp(app).get("/sql/")
-        assert response.status_int == 200
 
+    assert response.status_int == 200
     assert len(tracked_requests) == 1
     spans = tracked_requests[0].complete_spans
     assert [s.operation for s in spans] == [
@@ -174,8 +184,8 @@ def test_sql_capture_backtrace(should_capture_backtrace, tracked_requests):
     should_capture_backtrace.return_value = True
     with app_with_scout() as app:
         response = TestApp(app).get("/sql/")
-        assert response.status_int == 200
 
+    assert response.status_int == 200
     assert len(tracked_requests) == 1
     spans = tracked_requests[0].complete_spans
     assert [s.operation for s in spans] == [
@@ -190,8 +200,8 @@ def test_sql_capture_backtrace(should_capture_backtrace, tracked_requests):
 def test_template(tracked_requests):
     with app_with_scout() as app:
         response = TestApp(app).get("/template/")
-        assert response.status_int == 200
 
+    assert response.status_int == 200
     assert len(tracked_requests) == 1
     spans = tracked_requests[0].complete_spans
     print([s.operation for s in spans])
@@ -208,8 +218,8 @@ def test_template(tracked_requests):
 def test_no_monitor(tracked_requests):
     with app_with_scout(SCOUT_MONITOR=False) as app:
         response = TestApp(app).get("/hello/")
-        assert response.status_int == 200
 
+    assert response.status_int == 200
     assert len(tracked_requests) == 0
 
 
diff --git a/tests/unit/core/test_config.py b/tests/unit/core/test_config.py
index 4a102cb..ab5508c 100644
--- a/tests/unit/core/test_config.py
+++ b/tests/unit/core/test_config.py
@@ -3,6 +3,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 
 import os
 import re
+import sys
 
 import pytest
 
@@ -28,6 +29,12 @@ def test_get_config_value_from_python():
         ScoutConfig.reset_all()
 
 
+def test_unset_config_value_from_python():
+    ScoutConfig.set(revision_sha="foobar")
+    ScoutConfig.unset("revision_sha")
+    assert ScoutConfig().value("revision_sha") == ""  # from defaults
+
+
 def test_get_derived_config_value():
     ScoutConfig.set(core_agent_version="v1.1.8")
     config = ScoutConfig()
@@ -66,6 +73,22 @@ def test_get_undefined_config_value():
     assert config.value("unknown value") is None
 
 
+def test_get_undefined_config_value_null_layer_removed():
+    # For coverage
+    config = ScoutConfig()
+    config.layers = [x for x in config.layers if not isinstance(x, ScoutConfigNull)]
+
+    with pytest.raises(ValueError) as excinfo:
+        config.value("unknown value")
+
+    assert isinstance(excinfo.value, ValueError)
+    if sys.version_info[0] == 2:
+        expected_msg = "key u'unknown value' not found in any layer"
+    else:
+        expected_msg = "key 'unknown value' not found in any layer"
+    assert excinfo.value.args == (expected_msg,)
+
+
 def test_env_outranks_python():
     os.environ["SCOUT_SOCKET_PATH"] = "/set/in/env"
     ScoutConfig.set(socket_path="/set/from/python")