refactor: collect step artifacts immediately after step finish

tests/test_code_report.py

@@ -276,8 +276,8 @@ def test_uncrustify_file_diff(runner_with_analyzers: UniversumRunner,
 
     expected_log = log_success if expected_success else log_fail
     assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'"
-    expected_log = r"Collecting 'source_file.html' - [^\n]*Success" if expected_artifact \
-        else r"Collecting 'source_file.html' - [^\n]*Failed"
+    expected_artifacts_state = "Success" if expected_artifact else "Failed"
+    expected_log = f"Collecting artifacts for the 'Run uncrustify' step - [^\n]*{expected_artifacts_state}"
     assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'"
 
 

tests/test_integration.py

@@ -39,13 +39,14 @@ def test_artifacts(docker_main: UniversumRunner):
 files2 = Configuration([dict(name=" one/three/file.sh", command=["one/three/file.sh"])])
 
 artifacts = Configuration([dict(name="Existing artifacts", artifacts="one/**/file*", report_artifacts="one/*"),
-                        dict(name="Missing artifacts", artifacts="something", report_artifacts="something_else")])
+                           dict(name="Missing report artifacts", report_artifacts="non_existing_file"),
+                           dict(name="Missing all artifacts", artifacts="something", report_artifacts="something_else")])
 
 configs = mkdir * dirs1 + mkdir * dirs2 + mkfile * files1 + mkfile * files2 + artifacts
     """
     log = docker_main.run(config)
-    assert 'Failed' in get_line_with_text("Collecting 'something' - ", log)
-    assert 'Success' in get_line_with_text("Collecting 'something_else' for report - ", log)
+    assert 'Failed' in get_line_with_text("Collecting artifacts for the 'Missing all artifacts' step - ", log)
+    assert 'Success' in get_line_with_text("Collecting artifacts for the 'Missing report artifacts' step - ", log)
 
     assert os.path.exists(os.path.join(docker_main.artifact_dir, "three.zip"))
     assert os.path.exists(os.path.join(docker_main.artifact_dir, "two2.zip"))

universum/configuration_support.py

@@ -61,8 +61,8 @@ class Step:
         one time at most.
     artifacts
         Path to the file or directory to be copied to the working directory as an execution
-        result. Can contain shell-style pattern matching (e.g. `"out/*.html"`), including recursive wildcards
-        (e.g. `"out/**/index.html"`). If not stated otherwise (see ``--no-archive``
+        result immediately after step finish. Can contain shell-style pattern matching (e.g. `"out/*.html"`),
+        including recursive wildcards (e.g. `"out/**/index.html"`). If not stated otherwise (see ``--no-archive``
         command-line parameter for details), artifact directories are copied
         as archives. If `artifact_prebuild_clean` key is either absent or set to `False`
         and stated artifacts are present in downloaded sources, it is considered a failure and configuration

universum/main.py

@@ -87,7 +87,7 @@ def execute(self) -> None:
                     self.launcher.launch_custom_configs(afterall_configs)
                     self.code_report_collector.repo_diff = repo_diff
             self.code_report_collector.report_code_report_results()
-        self.artifacts.collect_artifacts()
+        self.artifacts.report_artifacts()
         self.reporter.report_build_result()
 
     def finalize(self) -> None:

universum/modules/artifact_collector.py

@@ -80,9 +80,6 @@ def __init__(self, *args, **kwargs):
         self.reporter = self.reporter_factory()
         self.automation_server = self.automation_server_factory()
 
-        self.artifact_list = []
-        self.report_artifact_list = []
-
         # Needed because of wildcards
         self.collected_report_artifacts = set()
 
@@ -126,7 +123,6 @@ def preprocess_artifact_list(self, artifact_list, ignore_already_existing=False)
         :param ignore_already_existing: will not check existence of artifacts when set to 'True'
         :return: sorted list of checked paths (including duplicates and wildcards)
         """
-        dir_list = set()
         for item in artifact_list:
             # Check existence in place: wildcards applied
             matches = glob2.glob(item["path"])
@@ -156,11 +152,6 @@ def preprocess_artifact_list(self, artifact_list, ignore_already_existing=False)
                 text += "\nPossible reason of this error: previous build results in working directory"
                 raise CriticalCiException(text)
 
-            dir_list.add(item["path"])
-        new_artifact_list = list(dir_list)
-        new_artifact_list.sort(key=len, reverse=True)
-        return new_artifact_list
-
     @make_block("Preprocessing artifact lists")
     def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existing_artifacts: bool = False) -> None:
         self.html_output.artifact_dir_ready = True
@@ -177,13 +168,12 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin
         if artifact_list:
             name = "Setting and preprocessing artifacts according to configs"
             with self.structure.block(block_name=name, pass_errors=True):
-                self.artifact_list = self.preprocess_artifact_list(artifact_list, ignore_existing_artifacts)
+                self.preprocess_artifact_list(artifact_list, ignore_existing_artifacts)
 
         if report_artifact_list:
             name = "Setting and preprocessing artifacts to be mentioned in report"
             with self.structure.block(block_name=name, pass_errors=True):
-                self.report_artifact_list = self.preprocess_artifact_list(report_artifact_list,
-                                                                          ignore_existing_artifacts)
+                self.preprocess_artifact_list(report_artifact_list, ignore_existing_artifacts)
 
     def move_artifact(self, path, is_report=False):
         self.out.log("Processing '" + path + "'")
@@ -221,18 +211,16 @@ def move_artifact(self, path, is_report=False):
                     artifact_path = self.automation_server.artifact_path(self.artifact_dir, artifact_name)
                     self.collected_report_artifacts.add(artifact_path)
 
-    @make_block("Collecting artifacts", pass_errors=False)
-    def collect_artifacts(self):
-        self.reporter.add_block_to_report(self.structure.get_current_block())
-        for path in self.report_artifact_list:
-            name = "Collecting '" + os.path.basename(path) + "' for report"
-            with self.structure.block(block_name=name, pass_errors=False):
-                self.move_artifact(path, is_report=True)
+    def collect_step_artifacts(self, step_artifacts, step_report_artifacts):
+        if step_artifacts:
+            path = utils.parse_path(step_artifacts, self.settings.project_root)
+            self.move_artifact(path, is_report=False)
+        if step_report_artifacts:
+            path = utils.parse_path(step_report_artifacts, self.settings.project_root)
+            self.move_artifact(path, is_report=True)
+
+    def report_artifacts(self):
         self.reporter.report_artifacts(list(self.collected_report_artifacts))
-        for path in self.artifact_list:
-            name = "Collecting '" + os.path.basename(path) + "'"
-            with self.structure.block(block_name=name, pass_errors=False):
-                self.move_artifact(path)
 
     def clean_artifacts_silently(self):
         try:

universum/modules/launcher.py

@@ -167,7 +167,8 @@ def __init__(self, item: configuration_support.Step,
                  log_file: Optional[TextIO],
                  working_directory: str,
                  additional_environment: Dict[str, str],
-                 background: bool) -> None:
+                 background: bool,
+                 artifact_collector_obj: artifact_collector.ArtifactCollector) -> None:
         super().__init__()
         self.configuration: configuration_support.Step = item
         self.out: Output = out
@@ -186,6 +187,8 @@ def __init__(self, item: configuration_support.Step,
         self._needs_finalization: bool = True
         self._error: Optional[str] = None
 
+        self.artifact_collector = artifact_collector_obj
+
     def prepare_command(self) -> bool:  # FIXME: refactor
         if not self.configuration.command:
             self.out.log("No 'command' found. Nothing to execute")
@@ -294,6 +297,10 @@ def finalize(self) -> None:
     def get_error(self) -> Optional[str]:
         return self._error
 
+    def collect_artifacts(self) -> None:
+        self.artifact_collector.collect_step_artifacts(self.configuration.artifacts,
+                                                       self.configuration.report_artifacts)
+
     def _handle_postponed_out(self) -> None:
         for item in self._postponed_out:
             item[0](item[1])
@@ -350,7 +357,7 @@ def __init__(self, *args, **kwargs) -> None:
         if not self.config_path:
             self.config_path = ".universum.py"
 
-        self.artifacts = self.artifacts_factory()
+        self.artifact_collector = self.artifacts_factory()
         self.api_support = self.api_support_factory()
         self.reporter = self.reporter_factory()
         self.server = self.server_factory()
@@ -370,7 +377,7 @@ def process_project_configs(self) -> configuration_support.Configuration:
             with open(config_path, encoding="utf-8") as config_file:
                 exec(config_file.read(), config_globals)  # pylint: disable=exec-used
             self.source_project_configs = config_globals["configs"]
-            dump_file: TextIO = self.artifacts.create_text_file("CONFIGS_DUMP.txt")
+            dump_file: TextIO = self.artifact_collector.create_text_file("CONFIGS_DUMP.txt")
             dump_file.write(self.source_project_configs.dump())
             dump_file.close()
             config = self.source_project_configs.filter(check_if_env_set)
@@ -413,12 +420,12 @@ def create_process(self, item: configuration_support.Step) -> RunningStep:
 
         log_file: Optional[TextIO] = None
         if self.output == "file":
-            log_file = self.artifacts.create_text_file(item.name + "_log.txt")
+            log_file = self.artifact_collector.create_text_file(item.name + "_log.txt")
             self.out.log("Execution log is redirected to file")
 
         additional_environment = self.api_support.get_environment_settings()
-        return RunningStep(item, self.out, self.server.add_build_tag,
-                    log_file, working_directory, additional_environment, item.background)
+        return RunningStep(item, self.out, self.server.add_build_tag, log_file, working_directory,
+                           additional_environment, item.background, self.artifact_collector)
 
     def launch_custom_configs(self, custom_configs: configuration_support.Configuration) -> None:
         self.structure.execute_step_structure(custom_configs, self.create_process)

universum/modules/structure_handler.py

@@ -85,6 +85,10 @@ def finalize(self) -> None:
     def get_error(self) -> Optional[str]:
         pass
 
+    @abstractmethod
+    def collect_artifacts(self) -> None:
+        pass
+
 
 class StructureHandler(HasOutput):
     def __init__(self, *args, **kwargs) -> None:
@@ -152,23 +156,20 @@ def block(self, *, block_name: str, pass_errors: bool) -> Generator:
             self.close_block()
 
     def execute_one_step(self, configuration: Step,
-                         step_executor: Callable[[Step], RunningStepBase]) -> Optional[str]:
+                         step_executor: Callable[[Step], RunningStepBase]) -> RunningStepBase:
         process: RunningStepBase = step_executor(configuration)
-
         process.start()
         if process.get_error() is not None:
-            return process.get_error()
-
+            return process
         if not configuration.background:
             process.finalize()
-            return process.get_error()  # could be None or error message
-
+            return process
         self.out.log("This step is marked to be executed in background")
         self.active_background_steps.append({'name': configuration.name,
                                              'block': self.get_current_block(),
                                              'process': process,
                                              'is_critical': configuration.critical})
-        return None
+        return process
 
     def finalize_background_step(self, background_step: BackgroundStepInfo) -> bool:
         process: RunningStepBase = background_step['process']
@@ -197,15 +198,22 @@ def process_one_step(self, merged_item: Step, step_executor: Callable, skip_exec
             self.report_skipped_block(numbering + "'" + merged_item.name + "'")
             return True
 
+        process = None
+        executed_successfully: bool = True
         # Here pass_errors=False, because any exception while executing build step
         # can be step-related and may not affect other steps
         with self.block(block_name=step_label, pass_errors=False):
-            error: Optional[str] = self.execute_one_step(merged_item, step_executor)
-            if error is not None:
+            process= self.execute_one_step(merged_item, step_executor)
+            error = process.get_error()
+            executed_successfully = (error is None)
+            if not executed_successfully:
+                error = error if error else ""
                 self.fail_current_block(error)
-                return False
+        if not merged_item.background:
+            with self.block(block_name=f"Collecting artifacts for the '{merged_item.name}' step", pass_errors=False):
+                process.collect_artifacts()
 
-        return True
+        return executed_successfully
 
     def execute_steps_recursively(self, parent: Step,
                                   children: Configuration,
@@ -261,6 +269,8 @@ def report_background_steps(self) -> bool:
                     result = False
                     self.out.report_skipped("The background step '" + item['name'] + "' failed, and as it is critical, "
                                             "all further steps will be skipped")
+            with self.block(block_name=f"Collecting artifacts for the '{item['name']}' step", pass_errors=False):
+                item['process'].collect_artifacts()
 
         self.out.log("All ongoing background steps completed")
         self.active_background_steps = []

universum/nonci.py

@@ -17,16 +17,16 @@ def __init__(self, *args, **kwargs):
 
     def execute(self):
         self.out.log("Cleaning artifacts...")
-        self.artifacts.clean_artifacts_silently()
+        self.artifact_collector.clean_artifacts_silently()
 
         project_configs = self.process_project_configs()
         self.code_report_collector.prepare_environment(project_configs)
-        self.artifacts.set_and_clean_artifacts(project_configs, ignore_existing_artifacts=True)
+        self.artifact_collector.set_and_clean_artifacts(project_configs, ignore_existing_artifacts=True)
 
         self.launch_project()
         self.reporter.report_initialized = True
+        self.artifact_collector.report_artifacts()
         self.reporter.report_build_result()
-        self.artifacts.collect_artifacts()
 
     def finalize(self):
         pass