Nicer pattern for make_app_client() in tests, closes #395

tests/fixtures.py

@@ -2,6 +2,7 @@
 from datasette.utils import sqlite3, MultiParams
 from asgiref.testing import ApplicationCommunicator
 from asgiref.sync import async_to_sync
+import contextlib
 from http.cookies import SimpleCookie
 import itertools
 import json
@@ -220,6 +221,7 @@ async def _request(
         return response
 
 
+@contextlib.contextmanager
 def make_app_client(
     sql_time_limit_ms=None,
     max_returned_rows=None,
@@ -281,7 +283,8 @@ def make_app_client(
 
 @pytest.fixture(scope="session")
 def app_client():
-    yield from make_app_client()
+    with make_app_client() as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
@@ -294,64 +297,75 @@ def app_client_no_files():
 
 @pytest.fixture(scope="session")
 def app_client_two_attached_databases():
-    yield from make_app_client(
+    with make_app_client(
         extra_databases={"extra database.db": EXTRA_DATABASE_SQL}
-    )
+    ) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_conflicting_database_names():
-    yield from make_app_client(
+    with make_app_client(
         extra_databases={"foo.db": EXTRA_DATABASE_SQL, "foo-bar.db": EXTRA_DATABASE_SQL}
-    )
+    ) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_two_attached_databases_one_immutable():
-    yield from make_app_client(
+    with make_app_client(
         is_immutable=True, extra_databases={"extra database.db": EXTRA_DATABASE_SQL}
-    )
+    ) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_with_hash():
-    yield from make_app_client(config={"hash_urls": True}, is_immutable=True)
+    with make_app_client(config={"hash_urls": True}, is_immutable=True) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_shorter_time_limit():
-    yield from make_app_client(20)
+    with make_app_client(20) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_returned_rows_matches_page_size():
-    yield from make_app_client(max_returned_rows=50)
+    with make_app_client(max_returned_rows=50) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_larger_cache_size():
-    yield from make_app_client(config={"cache_size_kb": 2500})
+    with make_app_client(config={"cache_size_kb": 2500}) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_csv_max_mb_one():
-    yield from make_app_client(config={"max_csv_mb": 1})
+    with make_app_client(config={"max_csv_mb": 1}) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_with_dot():
-    yield from make_app_client(filename="fixtures.dot.db")
+    with make_app_client(filename="fixtures.dot.db") as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_with_cors():
-    yield from make_app_client(cors=True)
+    with make_app_client(cors=True) as client:
+        yield client
 
 
 @pytest.fixture(scope="session")
 def app_client_immutable_and_inspect_file():
     inspect_data = {"fixtures": {"tables": {"sortable": {"count": 100}}}}
-    yield from make_app_client(is_immutable=True, inspect_data=inspect_data)
+    with make_app_client(is_immutable=True, inspect_data=inspect_data) as client:
+        yield client
 
 
 def generate_compound_rows(num):

tests/test_api.py

@@ -605,7 +605,7 @@ def test_invalid_custom_sql(app_client):
 
 
 def test_allow_sql_off():
-    for client in make_app_client(config={"allow_sql": False}):
+    with make_app_client(config={"allow_sql": False}) as client:
         response = client.get("/fixtures.json?sql=select+sleep(0.01)")
         assert 400 == response.status
         assert "sql= is not allowed" == response.json["error"]
@@ -1107,7 +1107,7 @@ def test_table_filter_extra_where_invalid(app_client):
 
 
 def test_table_filter_extra_where_disabled_if_no_sql_allowed():
-    for client in make_app_client(config={"allow_sql": False}):
+    with make_app_client(config={"allow_sql": False}) as client:
         response = client.get("/fixtures/facetable.json?_where=neighborhood='Dogpatch'")
         assert 400 == response.status
         assert "_where= is not allowed" == response.json["error"]
@@ -1528,14 +1528,14 @@ def test_suggested_facets(app_client):
 
 
 def test_allow_facet_off():
-    for client in make_app_client(config={"allow_facet": False}):
+    with make_app_client(config={"allow_facet": False}) as client:
         assert 400 == client.get("/fixtures/facetable.json?_facet=planet_int").status
         # Should not suggest any facets either:
         assert [] == client.get("/fixtures/facetable.json").json["suggested_facets"]
 
 
 def test_suggest_facets_off():
-    for client in make_app_client(config={"suggest_facets": False}):
+    with make_app_client(config={"suggest_facets": False}) as client:
         # Now suggested_facets should be []
         assert [] == client.get("/fixtures/facetable.json").json["suggested_facets"]
 
@@ -1667,7 +1667,7 @@ def test_config_cache_size(app_client_larger_cache_size):
 
 
 def test_config_force_https_urls():
-    for client in make_app_client(config={"force_https_urls": True}):
+    with make_app_client(config={"force_https_urls": True}) as client:
         response = client.get("/fixtures/facetable.json?_size=3&_facet=state")
         assert response.json["next_url"].startswith("https://")
         assert response.json["facet_results"]["state"]["results"][0][

tests/test_canned_write.py

@@ -4,7 +4,7 @@
 
 @pytest.fixture
 def canned_write_client():
-    for client in make_app_client(
+    with make_app_client(
         extra_databases={"data.db": "create table names (name text)"},
         metadata={
             "databases": {
@@ -35,7 +35,7 @@ def canned_write_client():
                 }
             }
         },
-    ):
+    ) as client:
         yield client
 
 

tests/test_cli.py

@@ -41,7 +41,7 @@ def test_inspect_cli_writes_to_file(app_client):
 
 def test_serve_with_inspect_file_prepopulates_table_counts_cache():
     inspect_data = {"fixtures": {"tables": {"hithere": {"count": 44}}}}
-    for client in make_app_client(inspect_data=inspect_data, is_immutable=True):
+    with make_app_client(inspect_data=inspect_data, is_immutable=True) as client:
         assert inspect_data == client.ds.inspect_data
         db = client.ds.databases["fixtures"]
         assert {"hithere": 44} == db.cached_table_counts

tests/test_custom_pages.py

@@ -27,7 +27,7 @@ def custom_pages_client(tmp_path_factory):
     nested_dir = pages_dir / "nested"
     nested_dir.mkdir()
     (nested_dir / "nest.html").write_text("Nest!", "utf-8")
-    for client in make_app_client(template_dir=str(template_dir)):
+    with make_app_client(template_dir=str(template_dir)) as client:
         yield client
 
 

tests/test_html.py

@@ -63,9 +63,9 @@ def test_static(app_client):
 
 
 def test_static_mounts():
-    for client in make_app_client(
+    with make_app_client(
         static_mounts=[("custom-static", str(pathlib.Path(__file__).parent))]
-    ):
+    ) as client:
         response = client.get("/custom-static/test_html.py")
         assert response.status == 200
         response = client.get("/custom-static/not_exists.py")
@@ -75,7 +75,7 @@ def test_static_mounts():
 
 
 def test_memory_database_page():
-    for client in make_app_client(memory=True):
+    with make_app_client(memory=True) as client:
         response = client.get("/:memory:")
         assert_permissions_checked(
             client.ds, ["view-instance", ("view-database", "database", ":memory:")]
@@ -177,7 +177,7 @@ def test_definition_sql(path, expected_definition_sql, app_client):
 
 
 def test_table_cell_truncation():
-    for client in make_app_client(config={"truncate_cells_html": 5}):
+    with make_app_client(config={"truncate_cells_html": 5}) as client:
         response = client.get("/fixtures/facetable")
         assert response.status == 200
         table = Soup(response.body, "html.parser").find("table")
@@ -202,7 +202,7 @@ def test_table_cell_truncation():
 
 
 def test_row_page_does_not_truncate():
-    for client in make_app_client(config={"truncate_cells_html": 5}):
+    with make_app_client(config={"truncate_cells_html": 5}) as client:
         response = client.get("/fixtures/facetable/1")
         assert response.status == 200
         assert_permissions_checked(
@@ -925,7 +925,7 @@ def test_table_metadata(app_client):
 
 
 def test_database_download_allowed_for_immutable():
-    for client in make_app_client(is_immutable=True):
+    with make_app_client(is_immutable=True) as client:
         assert not client.ds.databases["fixtures"].is_mutable
         # Regular page should have a download link
         response = client.get("/fixtures")
@@ -951,7 +951,7 @@ def test_database_download_disallowed_for_mutable(app_client):
 
 
 def test_database_download_disallowed_for_memory():
-    for client in make_app_client(memory=True):
+    with make_app_client(memory=True) as client:
         # Memory page should NOT have a download link
         response = client.get("/:memory:")
         soup = Soup(response.body, "html.parser")
@@ -960,7 +960,7 @@ def test_database_download_disallowed_for_memory():
 
 
 def test_allow_download_off():
-    for client in make_app_client(is_immutable=True, config={"allow_download": False}):
+    with make_app_client(is_immutable=True, config={"allow_download": False}) as client:
         response = client.get("/fixtures")
         soup = Soup(response.body, "html.parser")
         assert not len(soup.findAll("a", {"href": re.compile(r"\.db$")}))
@@ -978,7 +978,7 @@ def test_allow_sql_on(app_client):
 
 
 def test_allow_sql_off():
-    for client in make_app_client(config={"allow_sql": False}):
+    with make_app_client(config={"allow_sql": False}) as client:
         response = client.get("/fixtures")
         soup = Soup(response.body, "html.parser")
         assert not len(soup.findAll("textarea", {"name": "sql"}))
@@ -1170,9 +1170,9 @@ def test_metadata_json_html(app_client):
 
 
 def test_custom_table_include():
-    for client in make_app_client(
+    with make_app_client(
         template_dir=str(pathlib.Path(__file__).parent / "test_templates")
-    ):
+    ) as client:
         response = client.get("/fixtures/complex_foreign_keys")
         assert response.status == 200
         assert (
@@ -1197,7 +1197,7 @@ def test_zero_results(app_client, path):
 
 
 def test_config_template_debug_on():
-    for client in make_app_client(config={"template_debug": True}):
+    with make_app_client(config={"template_debug": True}) as client:
         response = client.get("/fixtures/facetable?_context=1")
         assert response.status == 200
         assert response.text.startswith("<pre>{")
@@ -1211,7 +1211,7 @@ def test_config_template_debug_off(app_client):
 
 def test_debug_context_includes_extra_template_vars():
     # https://github.com/simonw/datasette/issues/693
-    for client in make_app_client(config={"template_debug": True}):
+    with make_app_client(config={"template_debug": True}) as client:
         response = client.get("/fixtures/facetable?_context=1")
         # scope_path is added by PLUGIN1
         assert "scope_path" in response.text
@@ -1292,7 +1292,7 @@ def test_metadata_sort_desc(app_client):
     ],
 )
 def test_base_url_config(base_url, path):
-    for client in make_app_client(config={"base_url": base_url}):
+    with make_app_client(config={"base_url": base_url}) as client:
         response = client.get(base_url + path.lstrip("/"))
         soup = Soup(response.body, "html.parser")
         for el in soup.findAll(["a", "link", "script"]):

tests/test_plugins.py

@@ -229,9 +229,9 @@ def test_plugins_asgi_wrapper(app_client):
 
 
 def test_plugins_extra_template_vars(restore_working_directory):
-    for client in make_app_client(
+    with make_app_client(
         template_dir=str(pathlib.Path(__file__).parent / "test_templates")
-    ):
+    ) as client:
         response = client.get("/-/metadata")
         assert response.status == 200
         extra_template_vars = json.loads(
@@ -254,9 +254,9 @@ def test_plugins_extra_template_vars(restore_working_directory):
 
 
 def test_plugins_async_template_function(restore_working_directory):
-    for client in make_app_client(
+    with make_app_client(
         template_dir=str(pathlib.Path(__file__).parent / "test_templates")
-    ):
+    ) as client:
         response = client.get("/-/metadata")
         assert response.status == 200
         extra_from_awaitable_function = (