From 17144d84c2b63daf6e3aa9b42c6c773f134e8660 Mon Sep 17 00:00:00 2001 From: Tristan Maat Date: Wed, 18 Sep 2019 17:55:19 +0100 Subject: 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. --- src/buildstream/_loader/loader.py | 13 +++++++++---- src/buildstream/_project.py | 2 +- tests/internals/loader.py | 15 ++++++++------- tests/testutils/context.py | 9 +++++++++ 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 5eaed9337..a9e686f1c 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,7 @@ 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): for filename in targets: if os.path.isabs(filename): @@ -148,7 +153,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 +488,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 +598,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 . 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 -- cgit v1.2.1