From abc733912447f284b38ddc389d18ba0a8cef8bcf Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 7 Jun 2020 14:14:10 -0700 Subject: [PATCH] Nicer pattern for make_app_client() in tests, closes #395 --- tests/fixtures.py | 44 +++++++++++++++++++++++++------------- tests/test_api.py | 10 ++++----- tests/test_canned_write.py | 4 ++-- tests/test_cli.py | 2 +- tests/test_custom_pages.py | 2 +- tests/test_html.py | 28 ++++++++++++------------ tests/test_plugins.py | 8 +++---- 7 files changed, 56 insertions(+), 42 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index f767dc84..2ac73fb1 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -2,6 +2,7 @@ from datasette.app import Datasette 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 @@ class TestClient: 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): diff --git a/tests/test_api.py b/tests/test_api.py index 555e394a..22378946 100644 --- a/tests/test_api.py +++ b/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][ diff --git a/tests/test_canned_write.py b/tests/test_canned_write.py index 73b01e51..c217be8f 100644 --- a/tests/test_canned_write.py +++ b/tests/test_canned_write.py @@ -4,7 +4,7 @@ from .fixtures import make_app_client @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 diff --git a/tests/test_cli.py b/tests/test_cli.py index 2616f1d1..6939fe57 100644 --- a/tests/test_cli.py +++ b/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 diff --git a/tests/test_custom_pages.py b/tests/test_custom_pages.py index c69facb5..4e4b2a67 100644 --- a/tests/test_custom_pages.py +++ b/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 diff --git a/tests/test_html.py b/tests/test_html.py index ac7432d7..4e913bcf 100644 --- a/tests/test_html.py +++ b/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("
{")
@@ -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"]):
diff --git a/tests/test_plugins.py b/tests/test_plugins.py
index f69e7fa7..c782b87b 100644
--- a/tests/test_plugins.py
+++ b/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 = (