testutils/context.py: Mock tasks instead of accepting Nones

To ensure that we only disable element loading task progress reporting
for very specific code paths, we need to teach the test suite to be a
bit smarter.

For this reason we now mock a _Task object and return it in our mock
context's relevant method invocations.

Other code paths that deliberately invoke the loader without task
reporting now mark their loads with NO_PROGRESS.

4 files changed, 31 insertions, 12 deletions

diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 5eaed9337..ad4cd732a 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -37,6 +37,11 @@ from ..types import CoreWarnings
 from .._message import Message, MessageType
 
 
+# This should be used to deliberately disable progress reporting when
+# collecting an element
+_NO_PROGRESS = object()
+
+
 # Loader():
 #
 # The Loader class does the heavy lifting of parsing target
@@ -94,7 +99,11 @@ class Loader():
     # Raises: LoadError
     #
     # Returns: The toplevel LoadElement
-    def load(self, targets, rewritable=False, ticker=None, task=None):
+    def load(self, targets, task, rewritable=False, ticker=None):
+        # We ensure task is a real object here, since we previously
+        # allowed None, but would preferably like to fail if we still
+        # have a None.
+        assert task
 
         for filename in targets:
             if os.path.isabs(filename):
@@ -148,7 +157,7 @@ class Loader():
         self._clean_caches()
 
         # Cache how many Elements have just been loaded
-        if task:
+        if task is not _NO_PROGRESS:
             # Workaround for task potentially being None (because no State object)
             self.loaded = task.current_progress
 
@@ -483,7 +492,7 @@ class Loader():
 
         # Cache it now, make sure it's already there before recursing
         self._meta_elements[element.name] = meta_element
-        if task:
+        if task is not _NO_PROGRESS:
             task.add_current_progress()
 
         return meta_element
@@ -593,7 +602,7 @@ class Loader():
         # meta junction element
         # XXX: This is a likely point for progress reporting to end up
         # missing some elements, but it currently doesn't appear to be the case.
-        meta_element = self._collect_element_no_deps(self._elements[filename], None)
+        meta_element = self._collect_element_no_deps(self._elements[filename], _NO_PROGRESS)
         if meta_element.kind != 'junction':
             raise LoadError("{}{}: Expected junction but element kind is {}"
                             .format(provenance_str, filename, meta_element.kind),
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 6ae8aa9db..7ba93bba4 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -433,7 +433,7 @@ class Project():
     #
     def load_elements(self, targets, *, rewritable=False):
         with self._context.messenger.simple_task("Loading elements", silent_nested=True) as task:
-            meta_elements = self.loader.load(targets, rewritable=rewritable, ticker=None, task=task)
+            meta_elements = self.loader.load(targets, task, rewritable=rewritable, ticker=None)
 
         with self._context.messenger.simple_task("Resolving elements") as task:
             if task:
diff --git a/tests/internals/loader.py b/tests/internals/loader.py
index 9af2bf161..39ef8ac99 100644
--- a/tests/internals/loader.py
+++ b/tests/internals/loader.py
@@ -5,6 +5,7 @@ import pytest
 from buildstream._exceptions import LoadError, LoadErrorReason
 from buildstream._project import Project
 from buildstream._loader import MetaElement
+from buildstream._loader.loader import _NO_PROGRESS
 
 from tests.testutils import dummy_context
 
@@ -30,7 +31,7 @@ def test_one_file(datafiles):
 
     basedir = str(datafiles)
     with make_loader(basedir) as loader:
-        element = loader.load(['elements/onefile.bst'])[0]
+        element = loader.load(['elements/onefile.bst'], _NO_PROGRESS)[0]
 
         assert isinstance(element, MetaElement)
         assert element.kind == 'pony'
@@ -41,7 +42,7 @@ def test_missing_file(datafiles):
 
     basedir = str(datafiles)
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load(['elements/missing.bst'])
+        loader.load(['elements/missing.bst'], _NO_PROGRESS)
 
     assert exc.value.reason == LoadErrorReason.MISSING_FILE
 
@@ -51,7 +52,7 @@ def test_invalid_reference(datafiles):
 
     basedir = str(datafiles)
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load(['elements/badreference.bst'])
+        loader.load(['elements/badreference.bst'], _NO_PROGRESS)
 
     assert exc.value.reason == LoadErrorReason.INVALID_YAML
 
@@ -61,7 +62,7 @@ def test_invalid_yaml(datafiles):
 
     basedir = str(datafiles)
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load(['elements/badfile.bst'])
+        loader.load(['elements/badfile.bst'], _NO_PROGRESS)
 
     assert exc.value.reason == LoadErrorReason.INVALID_YAML
 
@@ -73,7 +74,7 @@ def test_fail_fullpath_target(datafiles):
     fullpath = os.path.join(basedir, 'elements', 'onefile.bst')
 
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load([fullpath])
+        loader.load([fullpath], _NO_PROGRESS)
 
     assert exc.value.reason == LoadErrorReason.INVALID_DATA
 
@@ -83,7 +84,7 @@ def test_invalid_key(datafiles):
 
     basedir = str(datafiles)
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load(['elements/invalidkey.bst'])
+        loader.load(['elements/invalidkey.bst'], _NO_PROGRESS)
 
     assert exc.value.reason == LoadErrorReason.INVALID_DATA
 
@@ -93,6 +94,6 @@ def test_invalid_directory_load(datafiles):
 
     basedir = str(datafiles)
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load(['elements/'])
+        loader.load(['elements/'], _NO_PROGRESS)
 
     assert exc.value.reason == LoadErrorReason.LOADING_DIRECTORY
diff --git a/tests/testutils/context.py b/tests/testutils/context.py
index 899bad247..849895e92 100644
--- a/tests/testutils/context.py
+++ b/tests/testutils/context.py
@@ -15,10 +15,13 @@
 #  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 
 import os
+from unittest.mock import MagicMock
 
+from types import MethodType
 from contextlib import contextmanager
 
 from buildstream._context import Context
+from buildstream._state import _Task
 
 
 # Handle messages from the pipeline
@@ -26,6 +29,11 @@ def _dummy_message_handler(message, is_silenced):
     pass
 
 
+@contextmanager
+def _get_dummy_task(self, activity_name, *, element_name=None, full_name=None, silent_nested=False):
+    yield MagicMock(spec=_Task("state", activity_name, full_name, 0))
+
+
 # dummy_context()
 #
 # Context manager to create minimal context for tests.
@@ -42,5 +50,6 @@ def dummy_context(*, config=None):
         context.load(config=config)
 
         context.messenger.set_message_handler(_dummy_message_handler)
+        context.messenger.simple_task = MethodType(_get_dummy_task, context.messenger)
 
         yield context