element.py: Don't reset a failed (but cached) workspaced Elementjennis/fix_failed_workspaces

A workspaced element can fail to build, if this happens, we
cache the result as a failure. This patch ensures that we
do not reset the cache data if a workspace has failed to build.

An integration test has also been added which ensures that we
are able to get the log of a failed workspace build.

Closes #1096

3 files changed, 47 insertions, 3 deletions

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 240e8ce7d..bc8cde311 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1199,7 +1199,7 @@ class Element(Plugin):
         # marked stable either when we verify that the workspace is already
         # cached, or when we build/pull the workspaced element.
         if self.__cache_keys_unstable:
-            if not self._cached_success():
+            if not self._cached():
                 self.__reset_cache_data()
                 if not self.__assemble_scheduled:
                     self._schedule_assemble()
@@ -1215,7 +1215,7 @@ class Element(Plugin):
         if (not self.__assemble_scheduled and not self.__assemble_done and
                 self.__artifact and
                 self._is_required() and
-                not self._cached_success() and
+                not self._cached() and
                 not self._pull_pending()):
             self._schedule_assemble()
 
@@ -3163,7 +3163,7 @@ class Element(Plugin):
     #
     def __update_cache_keys_stability(self):
         if self.__cache_keys_unstable:
-            if self._cached_success():
+            if self._cached():
                 self.__cache_keys_unstable = False
             elif not self.__assemble_scheduled and self.__assemble_done:
                 self.__cache_keys_unstable = False
diff --git a/tests/integration/project/elements/autotools/amhello-failure.bst b/tests/integration/project/elements/autotools/amhello-failure.bst
new file mode 100644
index 000000000..771466b49
--- /dev/null
+++ b/tests/integration/project/elements/autotools/amhello-failure.bst
@@ -0,0 +1,15 @@
+kind: autotools
+description: An element which is meant to fail
+
+config:
+  build-commands:
+    (>):
+    - this-will-fail
+
+depends:
+- base.bst
+
+sources:
+- kind: tar
+  url: project_dir:/files/amhello.tar.gz
+  ref: 9ba123fa4e660929e9a0aa99f0c487b7eee59c5e7594f3284d015640b90f5590
diff --git a/tests/integration/workspace.py b/tests/integration/workspace.py
index 045f8c490..fe5e76ea3 100644
--- a/tests/integration/workspace.py
+++ b/tests/integration/workspace.py
@@ -7,6 +7,7 @@ import pytest
 from buildstream import _yaml
 from buildstream.testing import cli_integration as cli  # pylint: disable=unused-import
 from buildstream.testing._utils.site import HAVE_SANDBOX
+from buildstream._exceptions import ErrorDomain
 
 
 pytestmark = pytest.mark.integration
@@ -314,3 +315,31 @@ def test_workspace_missing_last_successful(cli, datafiles):
     # Build again, ensure we dont crash just because the artifact went missing
     res = cli.run(project=project, args=['build', element_name])
     assert res.exit_code == 0
+
+
+# Check that we can still read failed workspace logs
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
+@pytest.mark.xfail(HAVE_SANDBOX == 'buildbox', reason='Not working with BuildBox', strict=True)
+def test_workspace_failed_logs(cli, datafiles):
+    project = str(datafiles)
+    workspace = os.path.join(cli.directory, 'failing_amhello')
+    element_name = 'autotools/amhello-failure.bst'
+
+    # Open workspace
+    res = cli.run(project=project, args=['workspace', 'open', '--directory', workspace, element_name])
+    res.assert_success()
+
+    # Try to build and ensure the build fails
+    res = cli.run(project=project, args=['build', element_name])
+    res.assert_main_error(ErrorDomain.STREAM, None)
+    assert cli.get_element_state(project, element_name) == 'failed'
+
+    res = cli.run(project=project, args=['artifact', 'log', element_name])
+    res.assert_success()
+
+    log = res.output
+    # Assert that we can get the log
+    assert log != ""
+    fail_str = "FAILURE {}: Running build-commands".format(element_name)
+    assert fail_str in log