Merge branch 'tpollard/945' into 'master'

Add initial TestArtifact() abstraction class to testutils

Closes #945

See merge request BuildStream/buildstream!1252

7 files changed, 129 insertions, 55 deletions

diff --git a/buildstream/_cas/cascache.py b/buildstream/_cas/cascache.py
index 19020e234..c739c7ee1 100644
--- a/buildstream/_cas/cascache.py
+++ b/buildstream/_cas/cascache.py
@@ -852,7 +852,7 @@ class CASCache():
                 a += 1
                 b += 1
 
-    def _reachable_refs_dir(self, reachable, tree, update_mtime=False):
+    def _reachable_refs_dir(self, reachable, tree, update_mtime=False, check_exists=False):
         if tree.hash in reachable:
             return
         try:
@@ -873,10 +873,13 @@ class CASCache():
         for filenode in directory.files:
             if update_mtime:
                 os.utime(self.objpath(filenode.digest))
+            if check_exists:
+                if not os.path.exists(self.objpath(filenode.digest)):
+                    raise FileNotFoundError
             reachable.add(filenode.digest.hash)
 
         for dirnode in directory.directories:
-            self._reachable_refs_dir(reachable, dirnode.digest, update_mtime=update_mtime)
+            self._reachable_refs_dir(reachable, dirnode.digest, update_mtime=update_mtime, check_exists=check_exists)
 
     def _required_blobs(self, directory_digest, *, excluded_subdirs=None):
         if not excluded_subdirs:
diff --git a/buildstream/plugintestutils/runcli.py b/buildstream/plugintestutils/runcli.py
index 1ddb95745..72bdce09e 100644
--- a/buildstream/plugintestutils/runcli.py
+++ b/buildstream/plugintestutils/runcli.py
@@ -52,7 +52,7 @@ from _pytest.capture import MultiCapture, FDCapture, FDCaptureBinary
 # Import the main cli entrypoint
 from buildstream._frontend import cli as bst_cli
 from buildstream import _yaml
-
+from buildstream._cas import CASCache
 
 # Special private exception accessor, for test case purposes
 from buildstream._exceptions import BstError, get_last_exception, get_last_task_error
@@ -253,6 +253,7 @@ class Cli():
         self.directory = directory
         self.config = None
         self.verbose = verbose
+        self.artifact = TestArtifact()
 
         if default_options is None:
             default_options = []
@@ -274,6 +275,15 @@ class Cli():
         for key, val in config.items():
             self.config[key] = val
 
+    # remove_artifact_from_cache():
+    #
+    # Remove given element artifact from artifact cache
+    #
+    # Args:
+    #    project (str): The project path under test
+    #    element_name (str): The name of the element artifact
+    #    cache_dir (str): Specific cache dir to remove artifact from
+    #
     def remove_artifact_from_cache(self, project, element_name,
                                    *, cache_dir=None):
         # Read configuration to figure out where artifacts are stored
@@ -285,10 +295,7 @@ class Cli():
             else:
                 cache_dir = default
 
-        cache_dir = os.path.join(cache_dir, 'cas', 'refs', 'heads')
-
-        cache_dir = os.path.splitext(os.path.join(cache_dir, 'test', element_name))[0]
-        shutil.rmtree(cache_dir)
+        self.artifact.remove_artifact_from_cache(cache_dir, element_name)
 
     # run():
     #
@@ -617,6 +624,101 @@ class CliRemote(CliIntegration):
         return configured_services
 
 
+class TestArtifact():
+
+    # remove_artifact_from_cache():
+    #
+    # Remove given element artifact from artifact cache
+    #
+    # Args:
+    #    cache_dir (str): Specific cache dir to remove artifact from
+    #    element_name (str): The name of the element artifact
+    #
+    def remove_artifact_from_cache(self, cache_dir, element_name):
+
+        cache_dir = os.path.join(cache_dir, 'cas', 'refs', 'heads')
+
+        cache_dir = os.path.splitext(os.path.join(cache_dir, 'test', element_name))[0]
+        shutil.rmtree(cache_dir)
+
+    # is_cached():
+    #
+    # Check if given element has a cached artifact
+    #
+    # Args:
+    #    cache_dir (str): Specific cache dir to check
+    #    element (Element): The element object
+    #    element_key (str): The element's cache key
+    #
+    # Returns:
+    #   (bool): If the cache contains the element's artifact
+    #
+    def is_cached(self, cache_dir, element, element_key):
+
+        cas = CASCache(str(cache_dir))
+        artifact_ref = element.get_artifact_name(element_key)
+        return cas.contains(artifact_ref)
+
+    # get_digest():
+    #
+    # Get the digest for a given element's artifact
+    #
+    # Args:
+    #    cache_dir (str): Specific cache dir to check
+    #    element (Element): The element object
+    #    element_key (str): The element's cache key
+    #
+    # Returns:
+    #   (Digest): The digest stored in the ref
+    #
+    def get_digest(self, cache_dir, element, element_key):
+
+        cas = CASCache(str(cache_dir))
+        artifact_ref = element.get_artifact_name(element_key)
+        digest = cas.resolve_ref(artifact_ref)
+        return digest
+
+    # extract_buildtree():
+    #
+    # Context manager for extracting an elements artifact buildtree for
+    # inspection.
+    #
+    # Args:
+    #    tmpdir (LocalPath): pytest fixture for the tests tmp dir
+    #    digest (Digest): The element directory digest to extract
+    #
+    # Yields:
+    #    (str): path to extracted buildtree directory, does not guarantee
+    #           existence.
+    @contextmanager
+    def extract_buildtree(self, tmpdir, digest):
+        with self._extract_subdirectory(tmpdir, digest, 'buildtree') as extract:
+            yield extract
+
+    # _extract_subdirectory():
+    #
+    # Context manager for extracting an element artifact for inspection,
+    # providing an expected path for a given subdirectory
+    #
+    # Args:
+    #    tmpdir (LocalPath): pytest fixture for the tests tmp dir
+    #    digest (Digest): The element directory digest to extract
+    #    subdir (str): Subdirectory to path
+    #
+    # Yields:
+    #    (str): path to extracted subdir directory, does not guarantee
+    #           existence.
+    @contextmanager
+    def _extract_subdirectory(self, tmpdir, digest, subdir):
+        with tempfile.TemporaryDirectory() as extractdir:
+            try:
+                cas = CASCache(str(tmpdir))
+                cas.checkout(extractdir, digest)
+                yield os.path.join(extractdir, subdir)
+            except FileNotFoundError:
+                yield None
+
+
 # Main fixture
 #
 # Use result = cli.run([arg1, arg2]) to run buildstream commands
diff --git a/tests/artifactcache/pull.py b/tests/artifactcache/pull.py
index 3c10c256c..d75b6d3c7 100644
--- a/tests/artifactcache/pull.py
+++ b/tests/artifactcache/pull.py
@@ -95,15 +95,14 @@ def test_pull(cli, tmpdir, datafiles):
         context.load(config=user_config_file)
         context.set_message_handler(message_handler)
 
-        # Load the project and CAS cache
+        # Load the project
         project = Project(project_dir, context)
         project.ensure_fully_loaded()
-        cas = context.artifactcache
 
         # Assert that the element's artifact is **not** cached
         element = project.load_elements(['target.bst'])[0]
         element_key = cli.get_element_key(project_dir, 'target.bst')
-        assert not cas.contains(element, element_key)
+        assert not cli.artifact.is_cached(cache_dir, element, element_key)
 
         queue = multiprocessing.Queue()
         # Use subprocess to avoid creation of gRPC threads in main BuildStream process
@@ -124,7 +123,7 @@ def test_pull(cli, tmpdir, datafiles):
             raise
 
         assert not error
-        assert cas.contains(element, element_key)
+        assert cli.artifact.is_cached(cache_dir, element, element_key)
 
 
 def _test_pull(user_config_file, project_dir, cache_dir,
@@ -209,11 +208,10 @@ def test_pull_tree(cli, tmpdir, datafiles):
         # Assert that the element's artifact is cached
         element = project.load_elements(['target.bst'])[0]
         element_key = cli.get_element_key(project_dir, 'target.bst')
-        assert artifactcache.contains(element, element_key)
+        assert cli.artifact.is_cached(rootcache_dir, element, element_key)
 
         # Retrieve the Directory object from the cached artifact
-        artifact_ref = element.get_artifact_name(element_key)
-        artifact_digest = cas.resolve_ref(artifact_ref)
+        artifact_digest = cli.artifact.get_digest(rootcache_dir, element, element_key)
 
         queue = multiprocessing.Queue()
         # Use subprocess to avoid creation of gRPC threads in main BuildStream process
diff --git a/tests/artifactcache/push.py b/tests/artifactcache/push.py
index 56af50a0d..426d22f24 100644
--- a/tests/artifactcache/push.py
+++ b/tests/artifactcache/push.py
@@ -75,13 +75,10 @@ def test_push(cli, tmpdir, datafiles):
         project = Project(project_dir, context)
         project.ensure_fully_loaded()
 
-        # Create a local CAS cache handle
-        cas = context.artifactcache
-
         # Assert that the element's artifact is cached
         element = project.load_elements(['target.bst'])[0]
         element_key = cli.get_element_key(project_dir, 'target.bst')
-        assert cas.contains(element, element_key)
+        assert cli.artifact.is_cached(rootcache_dir, element, element_key)
 
         queue = multiprocessing.Queue()
         # Use subprocess to avoid creation of gRPC threads in main BuildStream process
diff --git a/tests/integration/artifact.py b/tests/integration/artifact.py
index f9470ed6b..0718de893 100644
--- a/tests/integration/artifact.py
+++ b/tests/integration/artifact.py
@@ -58,16 +58,6 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
             'cachedir': str(tmpdir)
         })
 
-        @contextmanager
-        def cas_extract_buildtree(digest):
-            extractdir = tempfile.mkdtemp(prefix="tmp", dir=str(tmpdir))
-            try:
-                cas = CASCache(str(tmpdir))
-                cas.checkout(extractdir, digest)
-                yield os.path.join(extractdir, 'buildtree')
-            finally:
-                utils._force_rmtree(extractdir)
-
         # Build autotools element with the default behavior of caching buildtrees
         # only when necessary. The artifact should be successfully pushed to the share1 remote
         # and cached locally with an 'empty' buildtree digest, as it's not a
@@ -80,7 +70,7 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
         # The buildtree dir should not exist, as we set the config to not cache buildtrees.
         cache_key = cli.get_element_key(project, element_name)
         elementdigest = share1.has_artifact('test', element_name, cache_key)
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert not os.path.isdir(buildtreedir)
 
         # Delete the local cached artifacts, and assert the when pulled with --pull-buildtrees
@@ -89,7 +79,7 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
         assert cli.get_element_state(project, element_name) != 'cached'
         result = cli.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name])
         assert element_name in result.get_pulled_elements()
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert not os.path.isdir(buildtreedir)
         shutil.rmtree(os.path.join(str(tmpdir), 'cas'))
 
@@ -99,7 +89,7 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
         # leading to no buildtreedir being extracted
         result = cli.run(project=project, args=['artifact', 'pull', element_name])
         assert element_name in result.get_pulled_elements()
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert not os.path.isdir(buildtreedir)
         shutil.rmtree(os.path.join(str(tmpdir), 'cas'))
 
@@ -116,7 +106,7 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
 
         # Cache key will be the same however the digest hash will have changed as expected, so reconstruct paths
         elementdigest = share2.has_artifact('test', element_name, cache_key)
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert os.path.isdir(buildtreedir)
             assert os.listdir(buildtreedir)
 
@@ -126,7 +116,7 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
         assert cli.get_element_state(project, element_name) != 'cached'
         result = cli.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name])
         assert element_name in result.get_pulled_elements()
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert os.path.isdir(buildtreedir)
             assert os.listdir(buildtreedir)
         shutil.rmtree(os.path.join(str(tmpdir), 'cas'))
@@ -144,6 +134,6 @@ def test_cache_buildtrees(cli, tmpdir, datafiles):
         assert cli.get_element_state(project, element_name) == 'cached'
         cache_key = cli.get_element_key(project, element_name)
         elementdigest = share3.has_artifact('test', element_name, cache_key)
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert os.path.isdir(buildtreedir)
             assert os.listdir(buildtreedir)
diff --git a/tests/integration/pullbuildtrees.py b/tests/integration/pullbuildtrees.py
index e8f84d50d..1a89f776a 100644
--- a/tests/integration/pullbuildtrees.py
+++ b/tests/integration/pullbuildtrees.py
@@ -51,18 +51,6 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles):
             'cache': {'cache-buildtrees': 'always'},
         })
 
-        @contextmanager
-        def cas_extract_buildtree(digest):
-            extractdir = tempfile.mkdtemp(prefix="tmp", dir=str(tmpdir))
-            try:
-                cas = CASCache(str(tmpdir))
-                cas.checkout(extractdir, digest)
-                yield os.path.join(extractdir, 'buildtree')
-            except FileNotFoundError:
-                yield None
-            finally:
-                utils._force_rmtree(extractdir)
-
         # Build autotools element, checked pushed, delete local
         result = cli2.run(project=project, args=['build', element_name])
         assert result.exit_code == 0
@@ -88,11 +76,11 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles):
         result = cli2.run(project=project, args=['artifact', 'pull', element_name])
         assert element_name in result.get_pulled_elements()
         elementdigest = share1.has_artifact('test', element_name, cli2.get_element_key(project, element_name))
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert not buildtreedir
         result = cli2.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name])
         assert element_name in result.get_pulled_elements()
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert os.path.isdir(buildtreedir)
         default_state(cli2, tmpdir, share1)
 
@@ -151,7 +139,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles):
         result = cli2.run(project=project, args=['--pull-buildtrees', 'artifact', 'push', element_name])
         assert "Attempting to fetch missing artifact buildtrees" in result.stderr
         assert element_name not in result.get_pulled_elements()
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert not buildtreedir
         assert element_name not in result.get_pushed_elements()
         assert not share3.has_artifact('test', element_name, cli2.get_element_key(project, element_name))
@@ -164,7 +152,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles):
         result = cli2.run(project=project, args=['--pull-buildtrees', 'artifact', 'push', element_name])
         assert "Attempting to fetch missing artifact buildtrees" in result.stderr
         assert element_name in result.get_pulled_elements()
-        with cas_extract_buildtree(elementdigest) as buildtreedir:
+        with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir:
             assert os.path.isdir(buildtreedir)
         assert element_name in result.get_pushed_elements()
         assert share3.has_artifact('test', element_name, cli2.get_element_key(project, element_name))
diff --git a/tests/testutils/artifactshare.py b/tests/testutils/artifactshare.py
index 0fca42a6c..6c484ceb7 100644
--- a/tests/testutils/artifactshare.py
+++ b/tests/testutils/artifactshare.py
@@ -149,13 +149,9 @@ class ArtifactShare():
             tree = self.cas.resolve_ref(artifact_key)
             reachable = set()
             try:
-                self.cas._reachable_refs_dir(reachable, tree, update_mtime=False)
+                self.cas._reachable_refs_dir(reachable, tree, update_mtime=False, check_exists=True)
             except FileNotFoundError:
                 return None
-            for digest in reachable:
-                object_name = os.path.join(self.cas.casdir, 'objects', digest[:2], digest[2:])
-                if not os.path.exists(object_name):
-                    return None
             return tree
         except CASError:
             return None