Merge pull request #3867 from boegel/robust_update_progress_bar

make update_progress_bar a bit more robust by just doing nothing if the corresponding progress bar was not started (and making stopping of a non-started progress bar fatal)

Author: branfosj

easybuild/tools/output.py

@@ -250,22 +250,15 @@ def extensions_progress_bar():
     return progress_bar
 
 
-def get_progress_bar(bar_type, size=None):
+def get_progress_bar(bar_type, ignore_cache=False, size=None):
     """
     Get progress bar of given type.
     """
-    progress_bar_types = {
-        PROGRESS_BAR_DOWNLOAD_ALL: download_all_progress_bar,
-        PROGRESS_BAR_DOWNLOAD_ONE: download_one_progress_bar,
-        PROGRESS_BAR_EXTENSIONS: extensions_progress_bar,
-        PROGRESS_BAR_EASYCONFIG: easyconfig_progress_bar,
-        STATUS_BAR: status_bar,
-    }
 
     if bar_type == PROGRESS_BAR_DOWNLOAD_ONE and not size:
-        pbar = download_one_progress_bar_unknown_size()
-    elif bar_type in progress_bar_types:
-        pbar = progress_bar_types[bar_type]()
+        pbar = download_one_progress_bar_unknown_size(ignore_cache=ignore_cache)
+    elif bar_type in PROGRESS_BAR_TYPES:
+        pbar = PROGRESS_BAR_TYPES[bar_type](ignore_cache=ignore_cache)
     else:
         raise EasyBuildError("Unknown progress bar type: %s", bar_type)
 
@@ -295,27 +288,31 @@ def start_progress_bar(bar_type, size, label=None):
 
 def update_progress_bar(bar_type, label=None, progress_size=1):
     """
-    Update progress bar of given type, add progress of given size.
+    Update progress bar of given type (if it was started), add progress of given size.
 
     :param bar_type: type of progress bar
     :param label: label for progress bar
     :param progress_size: amount of progress made
     """
-    (pbar, task_id) = _progress_bar_cache[bar_type]
-    if label:
-        pbar.update(task_id, description=label)
-    if progress_size:
-        pbar.update(task_id, advance=progress_size)
+    if bar_type in _progress_bar_cache:
+        (pbar, task_id) = _progress_bar_cache[bar_type]
+        if label:
+            pbar.update(task_id, description=label)
+        if progress_size:
+            pbar.update(task_id, advance=progress_size)
 
 
 def stop_progress_bar(bar_type, visible=False):
     """
     Stop progress bar of given type.
     """
-    (pbar, task_id) = _progress_bar_cache[bar_type]
-    pbar.stop_task(task_id)
-    if not visible:
-        pbar.update(task_id, visible=False)
+    if bar_type in _progress_bar_cache:
+        (pbar, task_id) = _progress_bar_cache[bar_type]
+        pbar.stop_task(task_id)
+        if not visible:
+            pbar.update(task_id, visible=False)
+    else:
+        raise EasyBuildError("Failed to stop %s progress bar, since it was never started?!", bar_type)
 
 
 def print_checks(checks_data):
@@ -384,3 +381,13 @@ def print_checks(checks_data):
             console_print(table)
         else:
             print('\n'.join(lines))
+
+
+# this constant must be defined at the end, since functions used as values need to be defined
+PROGRESS_BAR_TYPES = {
+    PROGRESS_BAR_DOWNLOAD_ALL: download_all_progress_bar,
+    PROGRESS_BAR_DOWNLOAD_ONE: download_one_progress_bar,
+    PROGRESS_BAR_EXTENSIONS: extensions_progress_bar,
+    PROGRESS_BAR_EASYCONFIG: easyconfig_progress_bar,
+    STATUS_BAR: status_bar,
+}

test/framework/output.py

@@ -31,9 +31,12 @@
 from unittest import TextTestRunner
 from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered
 
+import easybuild.tools.output
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import build_option, get_output_style, update_build_option
-from easybuild.tools.output import DummyRich, colorize, show_progress_bars, status_bar, use_rich
+from easybuild.tools.output import PROGRESS_BAR_EXTENSIONS, PROGRESS_BAR_TYPES
+from easybuild.tools.output import DummyRich, colorize, get_progress_bar, show_progress_bars
+from easybuild.tools.output import start_progress_bar, status_bar, stop_progress_bar, update_progress_bar, use_rich
 
 try:
     import rich.progress
@@ -138,6 +141,46 @@ def test_colorize(self):
 
         self.assertErrorRegex(EasyBuildError, "Unknown color: nosuchcolor", colorize, 'test', 'nosuchcolor')
 
+    def test_get_progress_bar(self):
+        """
+        Test get_progress_bar.
+        """
+        # restore default configuration to show progress bars (disabled to avoid mangled test output),
+        # to ensure we'll get actual Progress instances when Rich is available
+        update_build_option('show_progress_bar', True)
+
+        for pbar_type in PROGRESS_BAR_TYPES:
+            pbar = get_progress_bar(pbar_type, ignore_cache=True)
+            if HAVE_RICH:
+                self.assertTrue(isinstance(pbar, rich.progress.Progress))
+            else:
+                self.assertTrue(isinstance(pbar, DummyRich))
+
+    def test_get_start_update_stop_progress_bar(self):
+        """
+        Test starting/updating/stopping of progress bars.
+        """
+        # clear progress bar cache first, this test assumes we start with a clean slate
+        easybuild.tools.output._progress_bar_cache.clear()
+
+        # restore default configuration to show progress bars (disabled to avoid mangled test output)
+        update_build_option('show_progress_bar', True)
+
+        # stopping a progress bar that never was started results in an error
+        error_pattern = "Failed to stop extensions progress bar, since it was never started"
+        self.assertErrorRegex(EasyBuildError, error_pattern, stop_progress_bar, PROGRESS_BAR_EXTENSIONS)
+
+        # updating a progress bar that never was started is silently ignored on purpose
+        update_progress_bar(PROGRESS_BAR_EXTENSIONS)
+        update_progress_bar(PROGRESS_BAR_EXTENSIONS, label="foo")
+        update_progress_bar(PROGRESS_BAR_EXTENSIONS, progress_size=100)
+
+        # also test normal cycle: start, update, stop
+        start_progress_bar(PROGRESS_BAR_EXTENSIONS, 100)
+        update_progress_bar(PROGRESS_BAR_EXTENSIONS)  # single step progress
+        update_progress_bar(PROGRESS_BAR_EXTENSIONS, label="test123", progress_size=5)
+        stop_progress_bar(PROGRESS_BAR_EXTENSIONS)
+
 
 def suite():
     """ returns all the testcases in this module """