Refactor cache-related fixtures

Add new fixtures which expose nicer interfaces for interacting with
the download caches. These are split into two variants -- one for
`refs/` and one for `downloads/` -- for the relevant functions. The
new fixtures cover getting the relevant cache dir as a path, getting
the expected cache location for a URI as a path (without creating it),
and injecting some specific file contents into said path, including
creation of the parent dir structure.

These new fixtures are then applied in as many locations as possible
for a small but valuable reduction in test complexity and tight
coupling between specific tests and the implementation.

Author: sirosen

tests/acceptance/test_nonjson_schema_handling.py

@@ -4,7 +4,6 @@
 import responses
 
 from check_jsonschema.parsers.json5 import ENABLED as JSON5_ENABLED
-from check_jsonschema.schema_loader.resolver import ref_url_to_cache_filename
 
 SIMPLE_SCHEMA = {
     "$schema": "http://json-schema.org/draft-07/schema",
@@ -138,7 +137,9 @@ def test_can_load_remote_yaml_schema_ref(run_line, tmp_path, passing_data):
     assert result.exit_code == (0 if passing_data else 1)
 
 
-def test_can_load_remote_yaml_schema_ref_from_cache(run_line, cache_dir, tmp_path):
+def test_can_load_remote_yaml_schema_ref_from_cache(
+    run_line, inject_cached_ref, tmp_path
+):
     retrieval_uri = "https://example.org/retrieval/schemas/main.yaml"
     responses.add(
         "GET",
@@ -155,10 +156,7 @@ def test_can_load_remote_yaml_schema_ref_from_cache(run_line, cache_dir, tmp_pat
     # populate a bad schema, but then "override" that with a good cache value
     # this can only pass (in the success case) if the cache loading really works
     responses.add("GET", ref_loc, body="false")
-    ref_cache_dir = cache_dir / "check_jsonschema" / "refs"
-    ref_cache_dir.mkdir(parents=True)
-    ref_path = ref_cache_dir / ref_url_to_cache_filename(ref_loc)
-    ref_path.write_text("type: string")
+    inject_cached_ref(ref_loc, "type: string")
 
     doc = tmp_path / "doc.json"
     doc.write_text(json.dumps(PASSING_DOCUMENT))

tests/acceptance/test_remote_ref_resolution.py

@@ -3,8 +3,6 @@
 import pytest
 import responses
 
-from check_jsonschema.schema_loader.resolver import ref_url_to_cache_filename
-
 CASES = {
     "case1": {
         "main_schema": {
@@ -68,7 +66,7 @@ def test_remote_ref_resolution_simple_case(run_line, check_passes, casename, tmp
 @pytest.mark.parametrize("casename", ("case1", "case2"))
 @pytest.mark.parametrize("disable_cache", (True, False))
 def test_remote_ref_resolution_cache_control(
-    run_line, tmp_path, cache_dir, casename, disable_cache
+    run_line, tmp_path, get_ref_cache_loc, casename, disable_cache
 ):
     main_schema_loc = "https://example.com/main.json"
     responses.add("GET", main_schema_loc, json=CASES[casename]["main_schema"])
@@ -92,9 +90,7 @@ def test_remote_ref_resolution_cache_control(
 
     cache_locs = []
     for ref_loc in ref_locs:
-        cache_locs.append(
-            cache_dir / "check_jsonschema" / "refs" / ref_url_to_cache_filename(ref_loc)
-        )
+        cache_locs.append(get_ref_cache_loc(ref_loc))
     assert cache_locs  # sanity check
     if disable_cache:
         for loc in cache_locs:
@@ -107,15 +103,11 @@ def test_remote_ref_resolution_cache_control(
 @pytest.mark.parametrize("casename", ("case1", "case2"))
 @pytest.mark.parametrize("check_passes", (True, False))
 def test_remote_ref_resolution_loads_from_cache(
-    run_line, tmp_path, cache_dir, casename, check_passes
+    run_line, tmp_path, get_ref_cache_loc, inject_cached_ref, casename, check_passes
 ):
     main_schema_loc = "https://example.com/main.json"
     responses.add("GET", main_schema_loc, json=CASES[casename]["main_schema"])
 
-    # ensure the ref cache dir exists
-    ref_cache_dir = cache_loc = cache_dir / "check_jsonschema" / "refs"
-    ref_cache_dir.mkdir(parents=True)
-
     ref_locs = []
     cache_locs = []
     for name, subschema in CASES[casename]["other_schemas"].items():
@@ -125,9 +117,8 @@ def test_remote_ref_resolution_loads_from_cache(
         ref_locs.append(other_schema_loc)
 
         # but populate the cache with "good data"
-        cache_loc = ref_cache_dir / ref_url_to_cache_filename(other_schema_loc)
-        cache_locs.append(cache_loc)
-        cache_loc.write_text(json.dumps(subschema))
+        inject_cached_ref(other_schema_loc, json.dumps(subschema))
+        cache_locs.append(get_ref_cache_loc(other_schema_loc))
 
     instance_path = tmp_path / "instance.json"
     instance_path.write_text(

tests/conftest.py

@@ -60,3 +60,53 @@ def patch_cache_dir(monkeypatch, cache_dir):
             "check_jsonschema.cachedownloader._base_cache_dir", lambda: str(cache_dir)
         )
         yield m
+
+
+@pytest.fixture
+def downloads_cache_dir(tmp_path):
+    return tmp_path / ".cache" / "check_jsonschema" / "downloads"
+
+
+@pytest.fixture
+def get_download_cache_loc(downloads_cache_dir):
+    def _get(uri):
+        return downloads_cache_dir / uri.split("/")[-1]
+
+    return _get
+
+
+@pytest.fixture
+def inject_cached_download(downloads_cache_dir, get_download_cache_loc):
+    def _write(uri, content):
+        downloads_cache_dir.mkdir(parents=True)
+        path = get_download_cache_loc(uri)
+        if isinstance(content, str):
+            path.write_text(content)
+        else:
+            path.write_bytes(content)
+
+    return _write
+
+
+@pytest.fixture
+def refs_cache_dir(tmp_path):
+    return tmp_path / ".cache" / "check_jsonschema" / "refs"
+
+
+@pytest.fixture
+def get_ref_cache_loc(refs_cache_dir):
+    from check_jsonschema.schema_loader.resolver import ref_url_to_cache_filename
+
+    def _get(uri):
+        return refs_cache_dir / ref_url_to_cache_filename(uri)
+
+    return _get
+
+
+@pytest.fixture
+def inject_cached_ref(refs_cache_dir, get_ref_cache_loc):
+    def _write(uri, content):
+        refs_cache_dir.mkdir(parents=True)
+        get_ref_cache_loc(uri).write_text(content)
+
+    return _write

tests/unit/test_cachedownloader.py

@@ -125,9 +125,9 @@ def test_cachedownloader_cached_file(tmp_path, monkeypatch, default_response):
 
 
 @pytest.mark.parametrize("disable_cache", (True, False))
-def test_cachedownloader_on_success(cache_dir, disable_cache):
+def test_cachedownloader_on_success(get_download_cache_loc, disable_cache):
     add_default_response()
-    f = cache_dir / "check_jsonschema" / "downloads" / "schema1.json"
+    f = get_download_cache_loc("schema1.json")
     cd = CacheDownloader(disable_cache=disable_cache).bind(
         "https://example.com/schema1.json"
     )
@@ -151,7 +151,9 @@ def test_cachedownloader_using_alternate_target_dir(cache_dir):
 
 @pytest.mark.parametrize("disable_cache", (True, False))
 @pytest.mark.parametrize("failures", (1, 2, requests.ConnectionError))
-def test_cachedownloader_succeeds_after_few_errors(cache_dir, disable_cache, failures):
+def test_cachedownloader_succeeds_after_few_errors(
+    get_download_cache_loc, disable_cache, failures
+):
     if isinstance(failures, int):
         for _i in range(failures):
             responses.add(
@@ -168,7 +170,7 @@ def test_cachedownloader_succeeds_after_few_errors(cache_dir, disable_cache, fai
             match_querystring=None,
         )
     add_default_response()
-    f = cache_dir / "check_jsonschema" / "downloads" / "schema1.json"
+    f = get_download_cache_loc("schema1.json")
     cd = CacheDownloader(disable_cache=disable_cache).bind(
         "https://example.com/schema1.json"
     )
@@ -184,7 +186,7 @@ def test_cachedownloader_succeeds_after_few_errors(cache_dir, disable_cache, fai
 @pytest.mark.parametrize("disable_cache", (True, False))
 @pytest.mark.parametrize("connection_error", (True, False))
 def test_cachedownloader_fails_after_many_errors(
-    cache_dir, disable_cache, connection_error
+    get_download_cache_loc, disable_cache, connection_error
 ):
     for _i in range(10):
         if connection_error:
@@ -202,7 +204,7 @@ def test_cachedownloader_fails_after_many_errors(
                 match_querystring=None,
             )
     add_default_response()  # never reached, the 11th response
-    f = cache_dir / "check_jsonschema" / "downloads" / "schema1.json"
+    f = get_download_cache_loc("schema1.json")
     cd = CacheDownloader(disable_cache=disable_cache).bind(
         "https://example.com/schema1.json"
     )
@@ -213,7 +215,7 @@ def test_cachedownloader_fails_after_many_errors(
 
 
 @pytest.mark.parametrize("disable_cache", (True, False))
-def test_cachedownloader_retries_on_bad_data(cache_dir, disable_cache):
+def test_cachedownloader_retries_on_bad_data(get_download_cache_loc, disable_cache):
     responses.add(
         "GET",
         "https://example.com/schema1.json",
@@ -222,7 +224,7 @@ def test_cachedownloader_retries_on_bad_data(cache_dir, disable_cache):
         match_querystring=None,
     )
     add_default_response()
-    f = cache_dir / "check_jsonschema" / "downloads" / "schema1.json"
+    f = get_download_cache_loc("schema1.json")
     cd = CacheDownloader(
         disable_cache=disable_cache,
     ).bind(
@@ -245,20 +247,19 @@ def test_cachedownloader_retries_on_bad_data(cache_dir, disable_cache):
     "failure_mode", ("header_missing", "header_malformed", "time_overflow")
 )
 def test_cachedownloader_handles_bad_lastmod_header(
-    monkeypatch, cache_dir, file_exists, failure_mode
+    monkeypatch,
+    get_download_cache_loc,
+    inject_cached_download,
+    file_exists,
+    failure_mode,
 ):
+    uri = "https://example.com/schema1.json"
     if failure_mode == "header_missing":
-        responses.add(
-            "GET",
-            "https://example.com/schema1.json",
-            headers={},
-            json={},
-            match_querystring=None,
-        )
+        responses.add("GET", uri, headers={}, json={}, match_querystring=None)
     elif failure_mode == "header_malformed":
         responses.add(
             "GET",
-            "https://example.com/schema1.json",
+            uri,
             headers={"Last-Modified": "Jan 2000 00:00:01"},
             json={},
             match_querystring=None,
@@ -274,47 +275,47 @@ def fake_mktime(*args):
         raise NotImplementedError
 
     original_file_contents = b'{"foo": "bar"}'
-    (cache_dir / "check_jsonschema" / "downloads").mkdir(parents=True)
-    f = cache_dir / "check_jsonschema" / "downloads" / "schema1.json"
+    file_path = get_download_cache_loc(uri)
 
+    assert not file_path.exists()
     if file_exists:
-        f.write_bytes(original_file_contents)
-    else:
-        assert not f.exists()
+        inject_cached_download(uri, original_file_contents)
 
-    cd = CacheDownloader().bind("https://example.com/schema1.json", filename=str(f))
+    cd = CacheDownloader().bind(uri)
 
     # if the file already existed, it will not be overwritten by the cachedownloader
     # so the returned value for both the downloader and a direct file read should be the
     # original contents
     if file_exists:
         with cd.open() as fp:
             assert fp.read() == original_file_contents
-        assert f.read_bytes() == original_file_contents
+        assert file_path.read_bytes() == original_file_contents
     # otherwise, the file will have been created with new content
     # both reads will show that new content
     else:
         with cd.open() as fp:
             assert fp.read() == b"{}"
-        assert f.read_bytes() == b"{}"
+        assert file_path.read_bytes() == b"{}"
 
     # at the end, the file always exists on disk
-    assert f.exists()
+    assert file_path.exists()
 
 
-def test_cachedownloader_validation_is_not_invoked_on_hit(monkeypatch, cache_dir):
+def test_cachedownloader_validation_is_not_invoked_on_hit(
+    monkeypatch, inject_cached_download
+):
     """
     Regression test for https://github.com/python-jsonschema/check-jsonschema/issues/453
 
     This was a bug in which the validation callback was invoked eagerly, even on a cache
     hit. As a result, cache hits did not demonstrate their expected performance gain.
     """
+    uri = "https://example.com/schema1.json"
+
     # 1: construct some perfectly good data (it doesn't really matter what it is)
     add_default_response()
     # 2: put equivalent data on disk
-    (cache_dir / "check_jsonschema" / "downloads").mkdir(parents=True)
-    f = cache_dir / "check_jsonschema" / "downloads" / "schema1.json"
-    f.write_text("{}")
+    inject_cached_download(uri, "{}")
 
     # 3: construct a validator which marks that it ran in a variable
     validator_ran = False
@@ -327,7 +328,6 @@ def dummy_validate_bytes(data):
     # and use the above validation method
     cd = CacheDownloader().bind(
         "https://example.com/schema1.json",
-        filename=str(f),
         validation_callback=dummy_validate_bytes,
     )