_sourcecache.py: Fix reporting of capabilities for the source cache

This also adds a test to ensure we are correctly setting the cache up

2 files changed, 63 insertions, 12 deletions

diff --git a/src/buildstream/_sourcecache.py b/src/buildstream/_sourcecache.py
index cdbe5b9cf..28ad82831 100644
--- a/src/buildstream/_sourcecache.py
+++ b/src/buildstream/_sourcecache.py
@@ -24,7 +24,7 @@ from ._remote import BaseRemote
 from ._cas.casremote import BlobNotFound
 from .storage._casbaseddirectory import CasBasedDirectory
 from ._basecache import BaseCache
-from ._exceptions import CASError, CASRemoteError, SourceCacheError
+from ._exceptions import CASError, CASRemoteError, SourceCacheError, RemoteError
 from . import utils
 from ._protos.buildstream.v2 import buildstream_pb2, buildstream_pb2_grpc, \
     source_pb2, source_pb2_grpc
@@ -48,10 +48,10 @@ class SourceRemote(BaseRemote):
     #
     # Check if this remote provides everything required for the
     # particular kind of remote. This is expected to be called as part
-    # of check(), and must be called in a non-main process.
+    # of check()
     #
-    # Returns:
-    #    (str|None): An error message, or None if no error message.
+    # Raises:
+    #     RemoteError: If the upstream has a problem
     #
     def _check(self):
         capabilities_service = buildstream_pb2_grpc.CapabilitiesStub(self.channel)
@@ -65,18 +65,15 @@ class SourceRemote(BaseRemote):
         except grpc.RpcError as e:
             # Check if this remote has the artifact service
             if e.code() == grpc.StatusCode.UNIMPLEMENTED:
-                return ("Configured remote does not have the BuildStream "
-                        "capabilities service. Please check remote configuration.")
-            # Else raise exception with details
-            return "Remote initialisation failed: {}".format(e.details())
+                raise RemoteError("Configured remote does not have the BuildStream "
+                                  "capabilities service. Please check remote configuration.")
+            raise RemoteError("Remote initialisation failed: {}".format(e.details()))
 
         if not response.source_capabilities:
-            return "Configured remote does not support source service"
+            raise RemoteError("Configured remote does not support source service")
 
         if self.spec.push and not response.source_capabilities.allow_updates:
-            return 'Source server does not allow push'
-
-        return None
+            raise RemoteError("Source server does not allow push")
 
     # get_source():
     #
diff --git a/tests/sourcecache/capabilities.py b/tests/sourcecache/capabilities.py
new file mode 100644
index 000000000..18dfddbab
--- /dev/null
+++ b/tests/sourcecache/capabilities.py
@@ -0,0 +1,54 @@
+# Pylint doesn't play well with fixtures and dependency injection from pytest
+# pylint: disable=redefined-outer-name
+
+import os
+
+import pytest
+from buildstream._project import Project
+
+from buildstream import _yaml
+from buildstream.testing.runcli import cli  # pylint: disable=unused-import
+from tests.testutils import dummy_context
+
+from tests.testutils.artifactshare import create_dummy_artifact_share
+
+
+DATA_DIR = os.path.join(
+    os.path.dirname(os.path.realpath(__file__)),
+    "project",
+)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_artifact_cache_with_missing_capabilities_is_skipped(cli, tmpdir, datafiles):
+    project_dir = str(datafiles)
+
+    # Set up an artifact cache.
+    with create_dummy_artifact_share() as share:
+        # Configure artifact share
+        cache_dir = os.path.join(str(tmpdir), 'cache')
+        user_config_file = str(tmpdir.join('buildstream.conf'))
+        user_config = {
+            'scheduler': {
+                'pushers': 1
+            },
+            'source-caches': {
+                'url': share.repo,
+            },
+            'cachedir': cache_dir
+        }
+        _yaml.roundtrip_dump(user_config, file=user_config_file)
+
+        with dummy_context(config=user_config_file) as context:
+            # Load the project
+            project = Project(project_dir, context)
+            project.ensure_fully_loaded()
+
+            # Create a local artifact cache handle
+            sourcecache = context.sourcecache
+
+            # Manually setup the CAS remote
+            sourcecache.setup_remotes(use_config=True)
+
+            assert not sourcecache.has_fetch_remotes(), \
+                "System didn't realize the source cache didn't support BuildStream"