refactor: collect step artifacts immediately after step finish

tests/test_code_report.py

@@ -276,8 +276,9 @@ 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/lib/ci_exception.py

@@ -21,4 +21,6 @@ def __init__(self, application_exit_code: int = 1) -> None:
 
 
 class StepException(Exception):
-    pass
+    def __init__(self, step_process=None):
+        super().__init__()
+        self.step_process = step_process

universum/main.py

@@ -87,7 +87,6 @@ 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.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
@@ -176,13 +167,13 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin
 
         if artifact_list:
             name = "Setting and preprocessing artifacts according to configs"
-            self.artifact_list = self.structure.run_in_block(self.preprocess_artifact_list,
-                                                             name, True, artifact_list, ignore_existing_artifacts)
+            self.structure.run_in_block(self.preprocess_artifact_list,
+                                        name, True, artifact_list, ignore_existing_artifacts)
         if report_artifact_list:
             name = "Setting and preprocessing artifacts to be mentioned in report"
-            self.report_artifact_list = self.structure.run_in_block(self.preprocess_artifact_list,
-                                                                    name, True, report_artifact_list,
-                                                                    ignore_existing_artifacts)
+            self.structure.run_in_block(self.preprocess_artifact_list,
+                                        name, True, report_artifact_list,
+                                        ignore_existing_artifacts)
 
     def move_artifact(self, path, is_report=False):
         self.out.log("Processing '" + path + "'")
@@ -220,16 +211,13 @@ 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"
-            self.structure.run_in_block(self.move_artifact, name, False, path, is_report=True)
-        self.reporter.report_artifacts(list(self.collected_report_artifacts))
-        for path in self.artifact_list:
-            name = "Collecting '" + os.path.basename(path) + "'"
-            self.structure.run_in_block(self.move_artifact, name, False, path)
+    def collect_step_artifacts(self, step):
+        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 clean_artifacts_silently(self):
         try:

universum/modules/launcher.py

@@ -168,7 +168,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,
+                 artifacts: artifact_collector.ArtifactCollector) -> None:
         super().__init__()
         self.configuration: configuration_support.Step = item
         self.out: Output = out
@@ -187,6 +188,8 @@ def __init__(self, item: configuration_support.Step,
         self._postponed_out: List[Tuple[Callable[[str], None], str]] = []
         self._needs_finalization: bool = True
 
+        self.artifacts = artifacts
+
     def prepare_command(self) -> bool:  # FIXME: refactor
         if not self.configuration.command:
             self.out.log("No 'command' found. Nothing to execute")
@@ -286,6 +289,9 @@ def finalize(self) -> None:
                 self.file.close()
             self._is_background = False
 
+    def collect_artifacts(self) -> None:
+        self.artifacts.collect_step_artifacts(self.configuration)
+
     def _handle_postponed_out(self) -> None:
         for item in self._postponed_out:
             item[0](item[1])
@@ -415,7 +421,7 @@ def fail_block(line: str = "") -> None:
 
         additional_environment = self.api_support.get_environment_settings()
         return RunningStep(item, self.out, fail_block, self.server.add_build_tag,
-                    log_file, working_directory, additional_environment, item.background)
+                    log_file, working_directory, additional_environment, item.background, self.artifacts)
 
     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

@@ -64,10 +64,12 @@ def is_successful(self) -> bool:
 class BackgroundStepInfo(TypedDict):
     name: str
     finalizer: Callable[[], None]
+    artifacts_collection: Callable
     is_critical: bool
 
 
 class StructureHandler(HasOutput):
+
     def __init__(self, *args, **kwargs) -> None:
         super().__init__(*args, **kwargs)
         self.current_block: Optional[Block] = Block("Universum")
@@ -138,15 +140,21 @@ def execute_one_step(self, configuration: Step,
         # step_executor is [[Step], Step], but referring to Step creates circular dependency
         process = step_executor(configuration)
 
-        process.start()
-        if not configuration.background:
-            process.finalize()
-            return
+        try:
+            process.start()
+            if not configuration.background:
+                process.finalize()
+                return process
+
+            self.out.log("This step is marked to be executed in background")
+            self.active_background_steps.append({'name': configuration.name,
+                                                 'finalizer': process.finalize,
+                                                 'artifacts_collection': process.collect_artifacts,
+                                                 'is_critical': is_critical})
 
-        self.out.log("This step is marked to be executed in background")
-        self.active_background_steps.append({'name': configuration.name,
-                                             'finalizer': process.finalize,
-                                             'is_critical': is_critical})
+            return process
+        except StepException as e:
+            raise StepException(process)
 
     def finalize_background_step(self, background_step: BackgroundStepInfo):
         try:
@@ -198,8 +206,16 @@ def execute_steps_recursively(self, parent: Optional[Step], cfg: Configuration,
 
                     # Here pass_errors=False, because any exception while executing build step
                     # can be step-related and may not affect other steps
-                    self.run_in_block(self.execute_one_step, step_name, False,
-                                      item, step_executor, obj_a.critical)
+                    step_process = None
+                    try:
+                        step_process = self.run_in_block(self.execute_one_step, step_name, False,
+                                                         item, step_executor, obj_a.critical)
+                        if not item.background:
+                            self._run_artifact_collection_step(item.name, step_process.collect_artifacts)
+                    except StepException as e:
+                        if not item.background:
+                            self._run_artifact_collection_step(item.name, e.step_process.collect_artifacts)
+                        raise e
             except StepException:
                 child_step_failed = True
                 if obj_a.critical:
@@ -211,9 +227,11 @@ def execute_steps_recursively(self, parent: Optional[Step], cfg: Configuration,
     def report_background_steps(self) -> bool:
         result = True
         for item in self.active_background_steps:
-            if not self.run_in_block(self.finalize_background_step,
+            finalization_result = self.run_in_block(self.finalize_background_step,
                                      "Waiting for background step '" + item['name'] + "' to finish...",
-                                     True, item):
+                                     True, item)
+            self._run_artifact_collection_step(item['name'], item['artifacts_collection'])
+            if not finalization_result:
                 result = False
         self.out.log("All ongoing background steps completed")
         self.active_background_steps = []
@@ -230,6 +248,11 @@ def execute_step_structure(self, configs: Configuration, step_executor) -> None:
         if self.active_background_steps:
             self.run_in_block(self.report_background_steps, "Reporting background steps", False)
 
+    def _run_artifact_collection_step(self, step_name, artifacts_collection_func):
+        artifacts_step_name = f"Collecting artifacts for the '{step_name}' step"
+        self.run_in_block(artifacts_collection_func, artifacts_step_name, pass_errors=False)
+
+
 
 class HasStructure(Module):
     structure_factory: ClassVar = Dependency(StructureHandler)

universum/nonci.py

@@ -26,7 +26,6 @@ def execute(self):
         self.launch_project()
         self.reporter.report_initialized = True
         self.reporter.report_build_result()
-        self.artifacts.collect_artifacts()
 
     def finalize(self):
         pass