WIP: Test caching and optimize parameterized resolver fixture

hab/cache.py

@@ -37,9 +37,7 @@ def __init__(self, site):
         self.enabled = True
 
         # Get the template filename used to find the cache files on disk
-        self.cache_template = self.site.get(
-            "site_cache_file_template", ["{stem}.habcache"]
-        )[0]
+        self.cache_template = self.site["site_cache_file_template"][0]
 
     @property
     def cached_keys(self):

hab/site.py

@@ -31,6 +31,7 @@ class Site(UserDict):
             "distro_paths": [],
             "ignored_distros": ["release", "pre"],
             "platforms": ["windows", "osx", "linux"],
+            "site_cache_file_template": ["{{stem}}.habcache"],
         }
     }
 

tests/conftest.py

@@ -8,25 +8,31 @@
 
 from hab import Resolver, Site
 
+# Testing both cached and uncached every time adds extra testing time. This env
+# var can be used to disable cached testing for local testing.
+if os.getenv("HAB_TEST_UNCACHED_ONLY", "0") == "1":
+    resolver_tests = ["uncached"]
+else:
+    resolver_tests = ["uncached", "cached"]
 
-@pytest.fixture
+
+@pytest.fixture(scope="session")
 def config_root():
     return Path(__file__).parent
 
 
-@pytest.fixture
-def habcached_site_file(config_root, tmpdir):
+def generate_habcached_site_file(config_root, dest):
     """Returns the path to a site config file generated from `site_main.json`
     configured so it can have a .habcache file generated next to it. The
     config_paths and distro_paths of the site file are hard coded to point to
     the repo's tests directory so it uses the same configs/distros. It also adds
     a `config-root` entry to `platform_path_maps`.
     """
-    site_file = Path(tmpdir) / "site.json"
+    site_file = Path(dest) / "site.json"
     site_src = config_root / "site_main.json"
 
     # Load the site_main.json files contents so we can modify it before saving
-    # it into the tmpdir for testing.
+    # it into the dest for testing.
     data = json.load(site_src.open())
     append = data["append"]
 
@@ -49,34 +55,62 @@ def habcached_site_file(config_root, tmpdir):
     return site_file
 
 
+@pytest.fixture(scope="session")
+def habcached_site_file(config_root, tmp_path_factory):
+    """Generates a site.json file and generates its habcache file.
+    This file is stored in a `_cache` directory in the pytest directory.
+    This persists for the entire testing session and can be used by other tests
+    that need to test hab when it is using a habcache.
+    """
+    # Create the site file
+    shared = tmp_path_factory.mktemp("_cache")
+    ret = generate_habcached_site_file(config_root, shared)
+
+    # Generate the habcache file
+    site = Site([ret])
+    resolver = Resolver(site)
+    site.cache.save_cache(resolver, ret)
+
+    return ret
+
+
 @pytest.fixture
 def habcached_resolver(habcached_site_file):
-    """Returns a Resolver using a habcache file.
+    """Returns a Resolver using a habcache file that was generated for this session.
 
     See the `habcached_site_file` fixture for details on how the cache is setup.
     For ease of testing the path to the saved habcache file is stored in
-    `_test_cache_file` on the returned resolver
-    """
-    """Returns the path to a site config file generated from `site_main.json`
-    configured so it can have a .habcache file generated next to it. The
-    config_paths and distro_paths of the site file are hard coded to point to
-    the repo's tests directory so it uses the same configs/distros. It also adds
-    a `config-root` entry to `platform_path_maps`.
+    `_test_cache_file` on the returned resolver.
     """
     site = Site([habcached_site_file])
     resolver = Resolver(site)
     # Generate the cache and provide easy access to the habcache file path
-    resolver._test_cache_file = site.cache.save_cache(resolver, habcached_site_file)
+    resolver._test_cache_file = site.cache.site_cache_path(habcached_site_file)
+
     return resolver
 
 
 @pytest.fixture
-def resolver(config_root):
-    """Return a standard testing resolver"""
+def uncached_resolver(config_root):
+    """Return a standard testing resolver not using any habcache files."""
     site = Site([config_root / "site_main.json"])
     return Resolver(site=site)
 
 
+@pytest.fixture(params=resolver_tests)
+def resolver(request):
+    """Returns a hab.Resolver instance using the site_main.json site config file.
+
+    This is a parameterized fixture that returns both cached and uncached versions
+    of the `site_main.json` site configuration. Note the cached version uses a
+    copy of it stored in the `_cache0` directory of the pytest temp files. This
+    should be used for most tests to ensure that all features are tested, but if
+    the test is not affected by caching you can use `uncached_resolver` instead.
+    """
+    test_map = {"uncached": "uncached_resolver", "cached": "habcached_resolver"}
+    return request.getfixturevalue(test_map[request.param])
+
+
 class Helpers(object):
     """A collection of reusable functions that tests can use."""
 

tests/test_cache.py

@@ -4,13 +4,13 @@
 from hab.parsers import Config, DistroVersion
 
 
-def test_cached_keys(resolver):
+def test_cached_keys(uncached_resolver):
     check = {
         "config_paths": ("*.json", Config),
         "distro_paths": ("*/.hab.json", DistroVersion),
     }
 
-    cache = resolver.site.cache
+    cache = uncached_resolver.site.cache
     # Container variable is not set by default
     assert not hasattr(cache, "_cached_keys")
     # On first call generates the correct return value
@@ -23,8 +23,8 @@ def test_cached_keys(resolver):
     assert cache.cached_keys == "Test value"
 
 
-def test_site_cache_path(config_root, resolver, tmpdir):
-    cache = resolver.site.cache
+def test_site_cache_path(config_root, uncached_resolver, tmpdir):
+    cache = uncached_resolver.site.cache
 
     # Test default
     site_file = Path(tmpdir) / "test.json"
@@ -52,7 +52,7 @@ def test_save_cache(config_root, tmpdir, habcached_resolver):
         assert cache[i] == check[i]
 
 
-def test_load_cache(config_root, resolver, habcached_site_file):
+def test_load_cache(config_root, uncached_resolver, habcached_site_file):
     """Tests non-cached resolved data matches a reference cached version."""
     cached_site = Site([habcached_site_file])
     cached_resolver = Resolver(cached_site)
@@ -64,7 +64,7 @@ def test_load_cache(config_root, resolver, habcached_site_file):
 
     # Check that un-cached resolver settings match the reference cache
     for key in ("config_paths", "distro_paths"):
-        assert getattr(resolver, key) == getattr(cached_resolver, key)
+        assert getattr(uncached_resolver, key) == getattr(cached_resolver, key)
 
 
 def test_cached_method(config_root, habcached_site_file):
@@ -90,3 +90,40 @@ def test_cached_method(config_root, habcached_site_file):
     result = cache.cache()
     assert isinstance(result, dict)
     assert not len(result)
+
+
+def test_resolver_cache(request, resolver):
+    """Tests that cached and uncached resolvers actually use/don't use the cache.
+
+    `uncached_resolver` should not have a habcache file and shouldn't use a cache.
+    `habcached_resolver` should have a habcache and uses it.
+    """
+    # Figure out if this is the cached or uncached resolver test
+    is_cached = "habcached_resolver" in request.fixturenames
+
+    # Get the site file path
+    assert len(resolver.site.paths) == 1
+    site_file = resolver.site.paths[0]
+    cache_file = resolver.site.cache.site_cache_path(site_file)
+
+    # The .habcache file should only exist when testing cached
+    if is_cached:
+        assert cache_file.exists()
+    else:
+        assert not cache_file.exists()
+
+    # force the resolver to load config/distro information
+    resolver.resolve("not_set")
+
+    cache = resolver.site.cache._cache
+    if is_cached:
+        # This habcache setup has exactly one cached glob string
+        assert len(cache["config_paths"]) == 1
+        assert len(cache["distro_paths"]) == 1
+        # The flat cache has many configs/distros, the test only needs to ensure
+        # that we have gotten some results
+        assert len(cache["flat"]["config_paths"]) > 10
+        assert len(cache["flat"]["distro_paths"]) > 10
+    else:
+        # If there aren't any habcache files, a default dict is returned
+        assert cache == {"flat": {"config_paths": {}, "distro_paths": {}}}

tests/test_formatter.py

@@ -53,9 +53,9 @@ def test_language_from_ext(monkeypatch):
         assert Formatter.language_from_ext("") == "sh"
 
 
-def test_format_environment_value(resolver):
+def test_format_environment_value(uncached_resolver):
     forest = {}
-    config = Config(forest, resolver)
+    config = Config(forest, uncached_resolver)
 
     # test_format_environment_value doesn't replace the special formatters.
     # This allows us to delay these formats to only when creating the final

tests/test_freeze.py

@@ -140,7 +140,7 @@ def test_unfreeze(config_root, resolver):
     assert cfg.alias_mods is NotSet
 
 
-def test_decode_freeze(config_root, resolver):
+def test_decode_freeze(config_root):
     check_file = config_root / "frozen_no_distros.json"
     checks = utils.json.load(check_file.open())
     v1 = checks["version1"]

tests/test_scripts.py

@@ -14,7 +14,7 @@
 
 
 @pytest.mark.parametrize("reference_name", reference_names)
-def test_scripts(resolver, tmpdir, monkeypatch, config_root, reference_name):
+def test_scripts(uncached_resolver, tmpdir, monkeypatch, config_root, reference_name):
     """Checks all of the scripts HabBase.write_script generates for a specified
     set of arguments.
 
@@ -62,7 +62,7 @@ def test_scripts(resolver, tmpdir, monkeypatch, config_root, reference_name):
     assert platform in ("linux", "osx", "win32")
     monkeypatch.setattr(utils, "Platform", utils.BasePlatform.get_platform(platform))
 
-    cfg = resolver.resolve(spec["uri"])
+    cfg = uncached_resolver.resolve(spec["uri"])
     reference = config_root / "reference_scripts" / reference_name
     ext = spec["ext"]
 
@@ -172,7 +172,7 @@ def walk_dir(current):
 
 
 @pytest.mark.skip(reason="Find a way to test complex alias evaluation in pytest")
-def test_complex_alias_bat(tmpdir, config_root, resolver):
+def test_complex_alias_bat(tmpdir, config_root):
     """This test is a placeholder for a future that can actually call hab's `hab.cli`
     and its aliases to check that they function correctly in Batch.
 
@@ -209,7 +209,7 @@ def test_complex_alias_bat(tmpdir, config_root, resolver):
 
 
 @pytest.mark.skip(reason="Find a way to test complex alias evaluation in pytest")
-def test_complex_alias_ps1(tmpdir, config_root, resolver):
+def test_complex_alias_ps1(tmpdir, config_root):
     """This test is a placeholder for a future that can actually call hab's `hab.cli`
     and its aliases to check that they function correctly in PowerShell.
 
@@ -246,7 +246,7 @@ def test_complex_alias_ps1(tmpdir, config_root, resolver):
 
 
 @pytest.mark.skip(reason="Find a way to test complex alias evaluation in pytest")
-def test_complex_alias_sh(tmpdir, config_root, resolver):
+def test_complex_alias_sh(tmpdir, config_root):
     """This test is a placeholder for a future that can actually call hab's `hab.cli`
     and its aliases to check that they function correctly in Bash.
 
@@ -283,14 +283,14 @@ def test_complex_alias_sh(tmpdir, config_root, resolver):
 
 
 @pytest.mark.parametrize("ext", (".bat", ".ps1", ".sh"))
-def test_invalid_alias(resolver, tmpdir, ext):
+def test_invalid_alias(uncached_resolver, tmpdir, ext):
     """Check that useful errors are raised if an invalid alias is passed or if
     the alias doesn't have "cmd" defined.
     """
     kwargs = dict(ext=ext, exit=True, args=None, create_launch=True)
 
     # Check that calling a bad alias name raises a useful error message
-    cfg = resolver.resolve("not_set/child")
+    cfg = uncached_resolver.resolve("not_set/child")
     with pytest.raises(errors.HabError, match=r'"bad-alias" is not a valid alias name'):
         cfg.write_script(str(tmpdir), launch="bad-alias", **kwargs)
 

tests/test_site.py

@@ -296,6 +296,8 @@ def test_dump_cached(config_root, habcached_site_file):
         "green": Fore.GREEN,
         "reset": Style.RESET_ALL,
     }
+    if platform != "windows":
+        check_template = check_template.replace("\\", "/")
 
     # With color enabled:
     # No verbosity, should not show cached status

tests/test_solver.py

@@ -62,14 +62,14 @@ def test_simplify_requirements(helpers, value, check):
         ),
     ),
 )
-def test_invalid_requirement_errors(resolver, requirements, match):
+def test_invalid_requirement_errors(uncached_resolver, requirements, match):
     """Test that the correct error is raised if an invalid or missing requirement
     is specified."""
     with pytest.raises(InvalidRequirementError, match=match):
-        resolver.resolve_requirements(requirements)
+        uncached_resolver.resolve_requirements(requirements)
 
 
-def test_solver_errors(resolver):
+def test_solver_errors(uncached_resolver):
     """Test that the correct errors are raised"""
 
     # Check that if we exceed max_redirects a MaxRedirectError is raised
@@ -83,7 +83,7 @@ def test_solver_errors(resolver):
         )
     )
 
-    solver = Solver(requirements, resolver)
+    solver = Solver(requirements, uncached_resolver)
     solver.max_redirects = 0
     with pytest.raises(MaxRedirectError, match="Redirect limit of 0 reached"):
         solver.resolve()