From 314879f70c5d4b7f04937d591c1bfd920bfe05cc Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 20 Oct 2022 07:35:58 +0300 Subject: [PATCH 01/25] feat(structure_handler): initialize conditional steps logic (#737) --- tests/test_conditional_steps.py | 66 ++++++++++++++++++++++++++ universum/configuration_support.py | 12 ++++- universum/modules/launcher.py | 31 ++++++------ universum/modules/structure_handler.py | 31 ++++++++++-- 4 files changed, 119 insertions(+), 21 deletions(-) create mode 100644 tests/test_conditional_steps.py diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py new file mode 100644 index 00000000..ca1fa92c --- /dev/null +++ b/tests/test_conditional_steps.py @@ -0,0 +1,66 @@ +import re +import inspect +import pytest + +from universum import __main__ + + +true_branch_step_name = "true_branch" +false_branch_step_name = "false_branch" + + +def test_conditional_true_branch(tmpdir, capsys): + check_conditional_step_success(tmpdir, capsys, conditional_step_passed=True) + + +def test_conditional_false_branch(tmpdir, capsys): + check_conditional_step_success(tmpdir, capsys, conditional_step_passed=False) + + +def check_conditional_step_success(tmpdir, capsys, conditional_step_passed): + config_file = build_config_file(tmpdir, conditional_step_passed) + check_conditional_step(tmpdir, capsys, config_file, conditional_step_passed) + + +def build_config_file(tmpdir, conditional_step_passed): + conditional_step_exit_code = 0 if conditional_step_passed else 1 + + config = inspect.cleandoc(f''' + from universum.configuration_support import Configuration, Step + + true_branch_step = Step(name='{true_branch_step_name}', command=['touch', '{true_branch_step_name}']) + false_branch_step = Step(name='{false_branch_step_name}', command=['touch', '{false_branch_step_name}']) + conditional_step = Configuration([dict(name='conditional', + command=['bash', '-c', 'exit {conditional_step_exit_code}'], + if_succeeded=true_branch_step, if_failed=false_branch_step)]) + + configs = conditional_step + ''') + + config_file = tmpdir.join("configs.py") + config_file.write_text(config, "utf-8") + + return config_file + + +def check_conditional_step(tmpdir, capsys, config_file, conditional_step_passed): + artifacts_dir = tmpdir.join("artifacts") + params = ["-vt", "none", + "-fsd", str(tmpdir), + "-ad", str(artifacts_dir), + "--clean-build", + "-o", "console"] + params.extend(["-cfg", str(config_file)]) + + return_code = __main__.main(params) + assert return_code == 0 + + captured = capsys.readouterr() + print(captured.out) + conditional_succeeded_regexp = r"\] conditional.*Success.*\| 5\.2" + assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) + + expected_log = true_branch_step_name if conditional_step_passed else false_branch_step_name + unexpected_log = false_branch_step_name if conditional_step_passed else true_branch_step_name + assert expected_log in captured.out + assert not unexpected_log in captured diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 3bdb8d11..9c5e4ac6 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -115,9 +115,9 @@ class Step: A tag used to mark successful TeamCity builds. This tag can be set independenty of `fail_tag` value per each step. The value should be set to a strings without spaces as acceptable by TeamCity as tags. Every tag is added (if matching condition) after executing build step it is set in, - not in the end of all run. + not in the end of all run. Not applicable for conditional steps. fail_tag - A tag used to mark failed TemCity builds. See `pass_tag` for details. + A tag used to mark failed TemCity builds. See `pass_tag` for details. Not applicable for conditional steps. Each parameter is optional, and is substituted with a falsy value, if omitted. @@ -170,6 +170,8 @@ def __init__(self, pass_tag: str = '', fail_tag: str = '', if_env_set: str = '', + if_succeeded = None, + if_failed = None, **kwargs) -> None: self.name: str = name self.directory: str = directory @@ -185,6 +187,9 @@ def __init__(self, self.pass_tag: str = pass_tag self.fail_tag: str = fail_tag self.if_env_set: str = if_env_set + self.if_succeeded = if_succeeded + self.if_failed = if_failed + self.is_conditional = self.if_succeeded or self.if_failed self.children: Optional['Configuration'] = None self._extras: Dict[str, str] = {} for key, value in kwargs.items(): @@ -391,6 +396,9 @@ def __add__(self, other: 'Step') -> 'Step': pass_tag=self.pass_tag + other.pass_tag, fail_tag=self.fail_tag + other.fail_tag, if_env_set=self.if_env_set + other.if_env_set, + # FIXME: This is a dummy implementation. Define addition logic and implement it. + if_succeeded=other.if_succeeded, + if_failed=other.if_failed, **combine(self._extras, other._extras) ) diff --git a/universum/modules/launcher.py b/universum/modules/launcher.py index e3e2bb6a..fe0b974e 100644 --- a/universum/modules/launcher.py +++ b/universum/modules/launcher.py @@ -248,16 +248,6 @@ def handle_stderr(self, line: str) -> None: else: self.out.log_stderr(line) - def add_tag(self, tag: str) -> None: - if not tag: - return - - request: Response = self.send_tag(tag) - if request.status_code != 200: - self.out.log_error(request.text) - else: - self.out.log("Tag '" + tag + "' added to build.") - def finalize(self) -> None: self._error = None if not self._needs_finalization: @@ -282,14 +272,12 @@ def finalize(self) -> None: text = utils.trim_and_convert_to_unicode(text) if self.file: self.file.write(text + "\n") - self.add_tag(self.configuration.fail_tag) self._error = text - return - - self.add_tag(self.configuration.pass_tag) - return finally: + tag: Optional[str] = self._get_teamcity_build_tag() + if tag: + self._assign_teamcity_build_tag(tag) self.handle_stdout() if self.file: self.file.close() @@ -307,6 +295,19 @@ def _handle_postponed_out(self) -> None: item[0](item[1]) self._postponed_out = [] + def _get_teamcity_build_tag(self) -> Optional[str]: + if self.configuration.is_conditional: + return None # conditional steps always succeed, no sense to set a tag + tag: str = self.configuration.fail_tag if self._error else self.configuration.pass_tag + return tag # can be also None if not set for current Configuration + + def _assign_teamcity_build_tag(self, tag: str) -> None: + response: Response = self.send_tag(tag) + if response.status_code != 200: + self.out.log_error(response.text) + else: + self.out.log("Tag '" + tag + "' added to build.") + class Launcher(ProjectDirectory, HasOutput, HasStructure, HasErrorState): artifacts_factory = Dependency(artifact_collector.ArtifactCollector) diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index f4758beb..783d22d0 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -148,10 +148,10 @@ def block(self, *, block_name: str, pass_errors: bool) -> Generator: yield except SilentAbortException: raise - except CriticalCiException as e: + except CriticalCiException as e: # system/environment step failed self.fail_current_block(str(e)) raise SilentAbortException() from e - except Exception as e: + except Exception as e: # unexpected failure, should not occur if pass_errors is True: raise self.fail_current_block(str(e)) @@ -244,12 +244,13 @@ def execute_steps_recursively(self, parent: Step, with self.block(block_name=step_label, pass_errors=True): current_step_failed = not self.execute_steps_recursively(merged_item, child.children, step_executor, skip_execution) + elif child.is_conditional: + current_step_failed = not self.execute_conditional_step(merged_item, step_executor) else: if merged_item.finish_background and self.active_background_steps: self.out.log("All ongoing background steps should be finished before next step execution") if not self.report_background_steps(): skip_execution = True - current_step_failed = not self.process_one_step(merged_item, step_executor, skip_execution) if current_step_failed: @@ -264,6 +265,20 @@ def execute_steps_recursively(self, parent: Step, return not some_step_failed + + def execute_conditional_step(self, step, step_executor): + self.configs_current_number += 1 + step_name = self._build_step_name(step.name) + conditional_step_succeeded = False + with self.block(block_name=step_name, pass_errors=True): + process = self.execute_one_step(step, step_executor) + conditional_step_succeeded = not process.get_error() + step_to_execute = step.if_succeeded if conditional_step_succeeded else step.if_failed + return self.execute_steps_recursively(parent=Step(), + children=Configuration([step_to_execute]), + step_executor=step_executor, + skip_execution=False) + def report_background_steps(self) -> bool: result: bool = True for item in self.active_background_steps: @@ -282,7 +297,10 @@ def report_background_steps(self) -> bool: return result def execute_step_structure(self, configs: Configuration, step_executor) -> None: - self.configs_total_count = sum(1 for _ in configs.all()) + for config in configs.all(): + self.configs_total_count += 1 + if config.is_conditional: + self.configs_total_count += 1 self.step_num_len = len(str(self.configs_total_count)) self.group_numbering = f" [ {'':{self.step_num_len}}+{'':{self.step_num_len}} ] " @@ -292,6 +310,11 @@ def execute_step_structure(self, configs: Configuration, step_executor) -> None: with self.block(block_name="Reporting background steps", pass_errors=False): self.report_background_steps() + def _build_step_name(self, name): + step_num_len = len(str(self.configs_total_count)) + numbering = f" [ {self.configs_current_number:>{step_num_len}}/{self.configs_total_count} ] " + return numbering + name + class HasStructure(Module): structure_factory: ClassVar = Dependency(StructureHandler) From 9525f51b298ac10fb6cd309130cb9cf1bbacd80a Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Mon, 21 Nov 2022 09:41:44 +0200 Subject: [PATCH 02/25] feat(artifact_collector): artifacts collection for conditional steps (#716) --- tests/test_conditional_steps.py | 96 +++++++++++++++++++------ universum/modules/artifact_collector.py | 39 ++++++++-- universum/modules/structure_handler.py | 24 +++---- 3 files changed, 115 insertions(+), 44 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index ca1fa92c..4a39584f 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -1,40 +1,81 @@ import re +import os import inspect -import pytest from universum import __main__ +from universum.configuration_support import Step -true_branch_step_name = "true_branch" -false_branch_step_name = "false_branch" +class StepsInfo: + conditional_step = None + true_branch_step = None + false_branch_step = None + is_conditional_step_passed = False def test_conditional_true_branch(tmpdir, capsys): - check_conditional_step_success(tmpdir, capsys, conditional_step_passed=True) + steps_info = get_conditional_steps_info(is_conditional_step_passed=True) + check_conditional_step(tmpdir, capsys, steps_info) def test_conditional_false_branch(tmpdir, capsys): - check_conditional_step_success(tmpdir, capsys, conditional_step_passed=False) + steps_info = get_conditional_steps_info(is_conditional_step_passed=False) + check_conditional_step(tmpdir, capsys, steps_info) -def check_conditional_step_success(tmpdir, capsys, conditional_step_passed): - config_file = build_config_file(tmpdir, conditional_step_passed) - check_conditional_step(tmpdir, capsys, config_file, conditional_step_passed) +# https://github.com/Samsung/Universum/issues/744 +# Artifact will be collected only from the second executed step, the first one will be overwritten +# Skipping checking file content to not overload tests with additional logic for incorrect behaviour check +def test_same_artifact(tmpdir, capsys): + steps_info = get_conditional_steps_info(is_conditional_step_passed=True) + conditional_step_artifact = steps_info.conditional_step["artifacts"] + steps_info.true_branch_step.command = ["touch", conditional_step_artifact] + steps_info.true_branch_step.artifacts = conditional_step_artifact -def build_config_file(tmpdir, conditional_step_passed): - conditional_step_exit_code = 0 if conditional_step_passed else 1 + check_conditional_step(tmpdir, capsys, steps_info) + +def get_conditional_steps_info(is_conditional_step_passed): + steps_info = StepsInfo() + + conditional_step_name = "conditional" + conditional_step_exit_code = 0 if is_conditional_step_passed else 1 + steps_info.conditional_step = Step( + name=conditional_step_name, + command=["bash", "-c", f"touch {conditional_step_name}; exit {conditional_step_exit_code}"], + artifacts=conditional_step_name) + steps_info.is_conditional_step_passed = is_conditional_step_passed + + true_branch_step_name = "true_branch" + steps_info.true_branch_step = Step( + name=true_branch_step_name, + command=["touch", true_branch_step_name], + artifacts=true_branch_step_name) + + false_branch_step_name = "false_branch" + steps_info.false_branch_step = Step( + name=false_branch_step_name, + command=["touch", false_branch_step_name], + artifacts=false_branch_step_name) + + return steps_info + + +def write_config_file(tmpdir, conditional_steps_info): config = inspect.cleandoc(f''' from universum.configuration_support import Configuration, Step - true_branch_step = Step(name='{true_branch_step_name}', command=['touch', '{true_branch_step_name}']) - false_branch_step = Step(name='{false_branch_step_name}', command=['touch', '{false_branch_step_name}']) - conditional_step = Configuration([dict(name='conditional', - command=['bash', '-c', 'exit {conditional_step_exit_code}'], - if_succeeded=true_branch_step, if_failed=false_branch_step)]) + true_branch_step = Step(**{str(conditional_steps_info.true_branch_step)}) + false_branch_step = Step(**{str(conditional_steps_info.false_branch_step)}) + conditional_step = Step(**{str(conditional_steps_info.conditional_step)}) + + # `true/false_branch_steps` should be Python objects from this script + conditional_step.is_conditional = True + conditional_step.if_succeeded = true_branch_step + conditional_step.if_failed = false_branch_step - configs = conditional_step + configs = Configuration([conditional_step]) ''') config_file = tmpdir.join("configs.py") @@ -43,7 +84,9 @@ def build_config_file(tmpdir, conditional_step_passed): return config_file -def check_conditional_step(tmpdir, capsys, config_file, conditional_step_passed): +def check_conditional_step(tmpdir, capsys, steps_info): + config_file = write_config_file(tmpdir, steps_info) + artifacts_dir = tmpdir.join("artifacts") params = ["-vt", "none", "-fsd", str(tmpdir), @@ -56,11 +99,20 @@ def check_conditional_step(tmpdir, capsys, config_file, conditional_step_passed) assert return_code == 0 captured = capsys.readouterr() - print(captured.out) conditional_succeeded_regexp = r"\] conditional.*Success.*\| 5\.2" assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) - expected_log = true_branch_step_name if conditional_step_passed else false_branch_step_name - unexpected_log = false_branch_step_name if conditional_step_passed else true_branch_step_name - assert expected_log in captured.out - assert not unexpected_log in captured + is_conditional_step_passed = steps_info.is_conditional_step_passed + conditional_step_artifact = steps_info.conditional_step.artifacts + true_branch_step_artifact = steps_info.true_branch_step.artifacts + false_branch_step_artifact = steps_info.false_branch_step.artifacts + + assert os.path.exists(os.path.join(artifacts_dir, conditional_step_artifact)) + + expected_artifact = true_branch_step_artifact if is_conditional_step_passed else false_branch_step_artifact + if expected_artifact: + assert os.path.exists(os.path.join(artifacts_dir, expected_artifact)) + + unexpected_artifact = false_branch_step_artifact if is_conditional_step_passed else true_branch_step_artifact + if unexpected_artifact: + assert not os.path.exists(os.path.join(artifacts_dir, unexpected_artifact)) diff --git a/universum/modules/artifact_collector.py b/universum/modules/artifact_collector.py index e6a5c094..cfdbcdf5 100644 --- a/universum/modules/artifact_collector.py +++ b/universum/modules/artifact_collector.py @@ -4,10 +4,11 @@ import os import shutil import zipfile +from typing import List, Optional, Dict, Union, TypedDict import glob2 -from ..configuration_support import Configuration +from ..configuration_support import Configuration, Step from ..lib.ci_exception import CriticalCiException, CiException from ..lib.gravity import Dependency from ..lib.utils import make_block @@ -58,6 +59,11 @@ def make_big_archive(target, source): return filename +class ArtifactInfo(TypedDict): + path: str + clean: bool + + class ArtifactCollector(ProjectDirectory, HasOutput, HasStructure): reporter_factory = Dependency(Reporter) automation_server_factory = Dependency(AutomationServerForHostingBuild) @@ -155,14 +161,15 @@ def preprocess_artifact_list(self, artifact_list, ignore_already_existing=False) @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 - artifact_list = [] - report_artifact_list = [] + artifact_list: List[ArtifactInfo] = [] + report_artifact_list: List[ArtifactInfo] = [] for configuration in project_configs.all(): if configuration.artifacts: - path = utils.parse_path(configuration.artifacts, self.settings.project_root) - artifact_list.append(dict(path=path, clean=configuration.artifact_prebuild_clean)) + artifact_list.append(self.get_config_artifact(configuration)) + if configuration.is_conditional: + artifact_list.extend(self.get_conditional_step_branches_artifacts(configuration)) if configuration.report_artifacts: - path = utils.parse_path(configuration.report_artifacts, self.settings.project_root) + path: str = utils.parse_path(configuration.report_artifacts, self.settings.project_root) report_artifact_list.append(dict(path=path, clean=configuration.artifact_prebuild_clean)) if artifact_list: @@ -175,6 +182,26 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin with self.structure.block(block_name=name, pass_errors=True): self.preprocess_artifact_list(report_artifact_list, ignore_existing_artifacts) + def get_conditional_step_branches_artifacts(self, step: Step) -> List[ArtifactInfo]: + succeeded_config_artifact: Optional[ArtifactInfo] = self.get_config_artifact_if_exists(step.if_succeeded) + failed_config_artifact: Optional[ArtifactInfo] = self.get_config_artifact_if_exists(step.if_failed) + + artifacts = [] + if succeeded_config_artifact: + artifacts.append(succeeded_config_artifact) + if failed_config_artifact: + artifacts.append(failed_config_artifact) + return artifacts + + def get_config_artifact_if_exists(self, step: Step) -> Optional[ArtifactInfo]: + if step and step.artifacts: + return self.get_config_artifact(step) + return None + + def get_config_artifact(self, step: Step) -> ArtifactInfo: + path = utils.parse_path(step.artifacts, self.settings.project_root) + return dict(path=path, clean=step.artifact_prebuild_clean) + def move_artifact(self, path, is_report=False): self.out.log("Processing '" + path + "'") matches = glob2.glob(path) diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index 783d22d0..73e51d8e 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -210,7 +210,7 @@ def process_one_step(self, merged_item: Step, step_executor: Callable, skip_exec with self.block(block_name=step_label, pass_errors=False): process = self.execute_one_step(merged_item, step_executor) error = process.get_error() - if error: + if error and not merged_item.is_conditional: self.fail_current_block(error) has_artifacts: bool = bool(merged_item.artifacts) or bool(merged_item.report_artifacts) if not merged_item.background and has_artifacts: @@ -245,7 +245,13 @@ def execute_steps_recursively(self, parent: Step, current_step_failed = not self.execute_steps_recursively(merged_item, child.children, step_executor, skip_execution) elif child.is_conditional: - current_step_failed = not self.execute_conditional_step(merged_item, step_executor) + conditional_step_succeeded: bool = self.process_one_step(merged_item, step_executor, + skip_execution=False) + step_to_execute: Step = merged_item.if_succeeded if conditional_step_succeeded else merged_item.if_failed + return self.execute_steps_recursively(parent=Step(), + children=Configuration([step_to_execute]), + step_executor=step_executor, + skip_execution=False) else: if merged_item.finish_background and self.active_background_steps: self.out.log("All ongoing background steps should be finished before next step execution") @@ -265,20 +271,6 @@ def execute_steps_recursively(self, parent: Step, return not some_step_failed - - def execute_conditional_step(self, step, step_executor): - self.configs_current_number += 1 - step_name = self._build_step_name(step.name) - conditional_step_succeeded = False - with self.block(block_name=step_name, pass_errors=True): - process = self.execute_one_step(step, step_executor) - conditional_step_succeeded = not process.get_error() - step_to_execute = step.if_succeeded if conditional_step_succeeded else step.if_failed - return self.execute_steps_recursively(parent=Step(), - children=Configuration([step_to_execute]), - step_executor=step_executor, - skip_execution=False) - def report_background_steps(self) -> bool: result: bool = True for item in self.active_background_steps: From 49b68214f8166825d73cffc3ce8d7f1fa9e0d27d Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Wed, 30 Nov 2022 11:29:31 +0200 Subject: [PATCH 03/25] feat(structure_handler): allow one branch step to be set in the conditional step (#749) --- tests/test_conditional_steps.py | 41 +++++++++++++++++--------- universum/modules/structure_handler.py | 14 +++++---- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 4a39584f..e0a33c90 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -1,6 +1,5 @@ import re import os -import inspect from universum import __main__ from universum.configuration_support import Step @@ -36,6 +35,18 @@ def test_same_artifact(tmpdir, capsys): check_conditional_step(tmpdir, capsys, steps_info) +def test_true_branch_chosen_but_absent(tmpdir, capsys): + steps_info = get_conditional_steps_info(is_conditional_step_passed=True) + steps_info.true_branch_step = None + check_conditional_step(tmpdir, capsys, steps_info) + + +def test_false_branch_chosen_but_absent(tmpdir, capsys): + steps_info = get_conditional_steps_info(is_conditional_step_passed=False) + steps_info.false_branch_step = None + check_conditional_step(tmpdir, capsys, steps_info) + + def get_conditional_steps_info(is_conditional_step_passed): steps_info = StepsInfo() @@ -63,20 +74,22 @@ def get_conditional_steps_info(is_conditional_step_passed): def write_config_file(tmpdir, conditional_steps_info): - config = inspect.cleandoc(f''' - from universum.configuration_support import Configuration, Step + true_branch_step = f"Step(**{str(conditional_steps_info.true_branch_step)})" if conditional_steps_info.true_branch_step else "None" + false_branch_step = f"Step(**{str(conditional_steps_info.false_branch_step)})" if conditional_steps_info.false_branch_step else "None" + config_lines = [ + "from universum.configuration_support import Configuration, Step", + f"true_branch_step = {true_branch_step}", + f"false_branch_step = {false_branch_step}", + f"conditional_step = Step(**{str(conditional_steps_info.conditional_step)})", - true_branch_step = Step(**{str(conditional_steps_info.true_branch_step)}) - false_branch_step = Step(**{str(conditional_steps_info.false_branch_step)}) - conditional_step = Step(**{str(conditional_steps_info.conditional_step)}) - # `true/false_branch_steps` should be Python objects from this script - conditional_step.is_conditional = True - conditional_step.if_succeeded = true_branch_step - conditional_step.if_failed = false_branch_step + "conditional_step.is_conditional = True", + "conditional_step.if_succeeded = true_branch_step", + "conditional_step.if_failed = false_branch_step", - configs = Configuration([conditional_step]) - ''') + "configs = Configuration([conditional_step])" + ] + config = "\n".join(config_lines) config_file = tmpdir.join("configs.py") config_file.write_text(config, "utf-8") @@ -104,8 +117,8 @@ def check_conditional_step(tmpdir, capsys, steps_info): is_conditional_step_passed = steps_info.is_conditional_step_passed conditional_step_artifact = steps_info.conditional_step.artifacts - true_branch_step_artifact = steps_info.true_branch_step.artifacts - false_branch_step_artifact = steps_info.false_branch_step.artifacts + true_branch_step_artifact = steps_info.true_branch_step.artifacts if steps_info.true_branch_step else None + false_branch_step_artifact = steps_info.false_branch_step.artifacts if steps_info.false_branch_step else None assert os.path.exists(os.path.join(artifacts_dir, conditional_step_artifact)) diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index 73e51d8e..8064a22a 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -247,11 +247,15 @@ def execute_steps_recursively(self, parent: Step, elif child.is_conditional: conditional_step_succeeded: bool = self.process_one_step(merged_item, step_executor, skip_execution=False) - step_to_execute: Step = merged_item.if_succeeded if conditional_step_succeeded else merged_item.if_failed - return self.execute_steps_recursively(parent=Step(), - children=Configuration([step_to_execute]), - step_executor=step_executor, - skip_execution=False) + step_to_execute: Step = merged_item.if_succeeded if conditional_step_succeeded \ + else merged_item.if_failed + if step_to_execute: # branch step can be not set + return self.execute_steps_recursively(parent=Step(), + children=Configuration([step_to_execute]), + step_executor=step_executor, + skip_execution=False) + else: # conditional step should be always successful + current_step_failed = False else: if merged_item.finish_background and self.active_background_steps: self.out.log("All ongoing background steps should be finished before next step execution") From c567a16b2b0580fe31f4ea44392e782ab0b04e3d Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Sat, 10 Dec 2022 15:35:12 +0200 Subject: [PATCH 04/25] feat(docs): overview of conditional steps --- doc/configuring.rst | 72 ++++++++++++++++++++++++++++++ universum/configuration_support.py | 8 +++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/doc/configuring.rst b/doc/configuring.rst index 2f610777..05ec6ffe 100644 --- a/doc/configuring.rst +++ b/doc/configuring.rst @@ -436,3 +436,75 @@ If executing the build step depends on more than one environment variable, use ` For example, ``if_env_set="SPECIAL_TOOL_PATH && ADDITIONAL_SOURCES_ROOT"`` step will be executed only in case of both `$SPECIAL_TOOL_PATH` and `$ADDITIONAL_SOURCES_ROOT` environment variables set to some values. If any of them is missing or not set in current environment, the step will be excluded from current run. + + +Conditional steps +--------------------- + +Conditional step is a :class:`Step` object, that has ``if_succeeded`` or ``if_failed`` parameters with other steps assigned. +If the conditional step succeeds, then the step from the ``if_succeeded`` parameter will be executed. +If the conditional step fails, the step from the ``if_failed`` parameter will be executed instead. + +Configuration example: + +.. testcode:: + + from universum.configuration_support import Configuration, Step + + true_branch_step = Step(name="Positive branch step", command=["ls"]) + false_branch_step = Step(name="Negative branch step", command=["pwd"]) + conditional_step = Step(name="Conditional step", command=["./script.sh"], + if_succeeded=true_branch_step, if_failed=false_branch_step) + + configs = Configuration([conditional_step]) + +Here's the example output for ``script.sh`` returning **zero** exit code: + +.. testoutput:: + + 5. Executing build steps + | 5.1. [ 1/2 ] Conditional step + | | ==> Adding file .../artifacts/Conditional_step_log.txt to artifacts... + | | ==> Execution log is redirected to file + | | $ .../temp/script.sh + | └ [Success] + | + | 5.2. [ 2/2 ] Positive branch step + | | ==> Adding file .../artifacts/Positive_branch_step_log.txt to artifacts... + | | ==> Execution log is redirected to file + | | $ /usr/bin/ls + | └ [Success] + | + └ [Success] + +Here's the example output for ``script.sh`` returning **non-zero** exit code: + +.. testoutput:: + + 5. Executing build steps + | 5.1. [ 1/2 ] Conditional step + | | ==> Adding file .../artifacts/Conditional_step_log.txt to artifacts... + | | ==> Execution log is redirected to file + | | $ .../temp/script.sh + | └ [Success] + | + | 5.2. [ 2/2 ] Negative branch step + | | ==> Adding file .../artifacts/Negative_branch_step_log.txt to artifacts... + | | ==> Execution log is redirected to file + | | $ /usr/bin/pwd + | └ [Success] + | + └ [Success] + +In general, conditional steps behave as any other regular steps, but here are some specifics: + +* Conditional step + * Will always be marked as successful in the log + * TeamCity tag will not be set for the conditional step +* Branch steps + * Only one branch step will be executed + * Both branches' artifacts will be checked for existence before the steps execution + * Artifacts collection or any other side-effects will not be triggered for non-executed branch step + * If chosen branch step is not set, nothing will happen. + For example, if the the ``Step.if_failed`` is not set and conditional step fails during the execution, nothing will happen. + * Only one branch step will be counted for each conditional step at calculating steps numbering and total count diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 9c5e4ac6..91bbaf93 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -117,7 +117,13 @@ class Step: TeamCity as tags. Every tag is added (if matching condition) after executing build step it is set in, not in the end of all run. Not applicable for conditional steps. fail_tag - A tag used to mark failed TemCity builds. See `pass_tag` for details. Not applicable for conditional steps. + A tag used to mark failed TeamCity builds. See `pass_tag` for details. Not applicable for conditional steps. + if_succeeded + Another step, that will be executed in case of this step will succeed. Having this parameter non-None will + make the current step conditional. + if_failed + Another step, that will be executed in case of this step will fail. Having this parameter non-None will + make the current step conditional. Each parameter is optional, and is substituted with a falsy value, if omitted. From 60c9f7bf5a276ef12a8c05c5029b0c9ce5097ac6 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 15 Dec 2022 19:52:31 +0200 Subject: [PATCH 05/25] test(artifact_collector): report artifacts for conditional steps --- tests/test_conditional_steps.py | 64 ++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index e0a33c90..31e63082 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -28,9 +28,9 @@ def test_conditional_false_branch(tmpdir, capsys): def test_same_artifact(tmpdir, capsys): steps_info = get_conditional_steps_info(is_conditional_step_passed=True) - conditional_step_artifact = steps_info.conditional_step["artifacts"] - steps_info.true_branch_step.command = ["touch", conditional_step_artifact] - steps_info.true_branch_step.artifacts = conditional_step_artifact + steps_info.true_branch_step.command = steps_info.conditional_step.command + steps_info.true_branch_step.artifacts = steps_info.conditional_step.artifacts + steps_info.true_branch_step.report_artifacts = steps_info.conditional_step.report_artifacts check_conditional_step(tmpdir, capsys, steps_info) @@ -51,28 +51,48 @@ def get_conditional_steps_info(is_conditional_step_passed): steps_info = StepsInfo() conditional_step_name = "conditional" + conditional_step_artifact = f"{conditional_step_name}_artifact" + conditional_step_report_artifact = f"{conditional_step_name}_report_artifact" conditional_step_exit_code = 0 if is_conditional_step_passed else 1 steps_info.conditional_step = Step( name=conditional_step_name, - command=["bash", "-c", f"touch {conditional_step_name}; exit {conditional_step_exit_code}"], - artifacts=conditional_step_name) + command=build_step_command(files_to_create=[conditional_step_artifact, conditional_step_report_artifact], + exit_code=conditional_step_exit_code), + artifacts=conditional_step_artifact, + report_artifacts=conditional_step_report_artifact) steps_info.is_conditional_step_passed = is_conditional_step_passed true_branch_step_name = "true_branch" + true_branch_step_artifact = f"{true_branch_step_name}_artifact" + true_branch_step_report_artifact = f"{true_branch_step_name}_report_artifact" steps_info.true_branch_step = Step( name=true_branch_step_name, - command=["touch", true_branch_step_name], - artifacts=true_branch_step_name) + command=build_step_command(files_to_create=[true_branch_step_artifact, true_branch_step_report_artifact], + exit_code=0), + artifacts=true_branch_step_artifact, + report_artifacts=true_branch_step_report_artifact) false_branch_step_name = "false_branch" + false_branch_step_artifact = f"{false_branch_step_name}_artifact" + false_branch_step_report_artifact = f"{false_branch_step_name}_report_artifact" steps_info.false_branch_step = Step( name=false_branch_step_name, - command=["touch", false_branch_step_name], - artifacts=false_branch_step_name) + command=build_step_command(files_to_create=[false_branch_step_artifact, false_branch_step_report_artifact], + exit_code=0), + artifacts=false_branch_step_artifact, + report_artifacts=false_branch_step_report_artifact) return steps_info +def build_step_command(files_to_create, exit_code): + commands = [] + for f in files_to_create: + commands.append(f"touch {f}") + commands.append(f"exit {exit_code}") + return ["bash", "-c", ";".join(commands)] + + def write_config_file(tmpdir, conditional_steps_info): true_branch_step = f"Step(**{str(conditional_steps_info.true_branch_step)})" if conditional_steps_info.true_branch_step else "None" false_branch_step = f"Step(**{str(conditional_steps_info.false_branch_step)})" if conditional_steps_info.false_branch_step else "None" @@ -116,16 +136,36 @@ def check_conditional_step(tmpdir, capsys, steps_info): assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) is_conditional_step_passed = steps_info.is_conditional_step_passed - conditional_step_artifact = steps_info.conditional_step.artifacts - true_branch_step_artifact = steps_info.true_branch_step.artifacts if steps_info.true_branch_step else None - false_branch_step_artifact = steps_info.false_branch_step.artifacts if steps_info.false_branch_step else None + conditional_step_artifact = get_step_artifact(steps_info.conditional_step) + conditional_step_report_artifact = get_step_report_artifact(steps_info.conditional_step) + true_branch_step_artifact = get_step_artifact(steps_info.true_branch_step) + true_branch_step_report_artifact = get_step_report_artifact(steps_info.true_branch_step) + false_branch_step_artifact = get_step_artifact(steps_info.false_branch_step) + false_branch_step_report_artifact = get_step_report_artifact(steps_info.false_branch_step) assert os.path.exists(os.path.join(artifacts_dir, conditional_step_artifact)) + assert os.path.exists(os.path.join(artifacts_dir, conditional_step_report_artifact)) expected_artifact = true_branch_step_artifact if is_conditional_step_passed else false_branch_step_artifact + expected_report_artifact = true_branch_step_report_artifact if is_conditional_step_passed \ + else false_branch_step_report_artifact if expected_artifact: assert os.path.exists(os.path.join(artifacts_dir, expected_artifact)) + if expected_report_artifact: + assert os.path.exists(os.path.join(artifacts_dir, expected_report_artifact)) unexpected_artifact = false_branch_step_artifact if is_conditional_step_passed else true_branch_step_artifact + unexpected_report_artifact = false_branch_step_report_artifact if is_conditional_step_passed \ + else true_branch_step_report_artifact if unexpected_artifact: assert not os.path.exists(os.path.join(artifacts_dir, unexpected_artifact)) + if unexpected_report_artifact: + assert not os.path.exists(os.path.join(artifacts_dir, unexpected_report_artifact)) + + +def get_step_artifact(step): + return step.artifacts if step else None + + +def get_step_report_artifact(step): + return step.report_artifacts if step else None From 293551ca3293253dd26d545004d5da673c1785d8 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Wed, 28 Dec 2022 16:35:03 +0200 Subject: [PATCH 06/25] test(artifact_collector): refactor conditional step artifacts checks (#752) --- tests/test_conditional_steps.py | 64 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 31e63082..11273bed 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -135,37 +135,33 @@ def check_conditional_step(tmpdir, capsys, steps_info): conditional_succeeded_regexp = r"\] conditional.*Success.*\| 5\.2" assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) - is_conditional_step_passed = steps_info.is_conditional_step_passed - conditional_step_artifact = get_step_artifact(steps_info.conditional_step) - conditional_step_report_artifact = get_step_report_artifact(steps_info.conditional_step) - true_branch_step_artifact = get_step_artifact(steps_info.true_branch_step) - true_branch_step_report_artifact = get_step_report_artifact(steps_info.true_branch_step) - false_branch_step_artifact = get_step_artifact(steps_info.false_branch_step) - false_branch_step_report_artifact = get_step_report_artifact(steps_info.false_branch_step) - - assert os.path.exists(os.path.join(artifacts_dir, conditional_step_artifact)) - assert os.path.exists(os.path.join(artifacts_dir, conditional_step_report_artifact)) - - expected_artifact = true_branch_step_artifact if is_conditional_step_passed else false_branch_step_artifact - expected_report_artifact = true_branch_step_report_artifact if is_conditional_step_passed \ - else false_branch_step_report_artifact - if expected_artifact: - assert os.path.exists(os.path.join(artifacts_dir, expected_artifact)) - if expected_report_artifact: - assert os.path.exists(os.path.join(artifacts_dir, expected_report_artifact)) - - unexpected_artifact = false_branch_step_artifact if is_conditional_step_passed else true_branch_step_artifact - unexpected_report_artifact = false_branch_step_report_artifact if is_conditional_step_passed \ - else true_branch_step_report_artifact - if unexpected_artifact: - assert not os.path.exists(os.path.join(artifacts_dir, unexpected_artifact)) - if unexpected_report_artifact: - assert not os.path.exists(os.path.join(artifacts_dir, unexpected_report_artifact)) - - -def get_step_artifact(step): - return step.artifacts if step else None - - -def get_step_report_artifact(step): - return step.report_artifacts if step else None + artifacts_checker = ConditionalStepsArtifactChecker(artifacts_dir, steps_info) + artifacts_checker.check_conditional_step_artifacts_present() + artifacts_checker.check_executed_step_artifacts_present() + artifacts_checker.check_not_executed_step_artifacts_absent() + + +class ConditionalStepsArtifactChecker: + + def __init__(self, artifacts_dir, steps_info): + self._artifacts_dir = artifacts_dir + self._conditional_step = steps_info.conditional_step + self._executed_step = steps_info.true_branch_step if steps_info.is_conditional_step_passed \ + else steps_info.false_branch_step + self._not_executed_step = steps_info.false_branch_step if steps_info.is_conditional_step_passed \ + else steps_info.true_branch_step + + def check_conditional_step_artifacts_present(self): + self._check_artifacts_presence(self._conditional_step, is_presence_expected=True) + + def check_executed_step_artifacts_present(self): + self._check_artifacts_presence(self._executed_step, is_presence_expected=True) + + def check_not_executed_step_artifacts_absent(self): + self._check_artifacts_presence(self._not_executed_step, is_presence_expected=False) + + def _check_artifacts_presence(self, step, is_presence_expected): + if not step: # branch step can be not set + return + for artifact in [step.artifacts, step.report_artifacts]: + assert os.path.exists(os.path.join(self._artifacts_dir, artifact)) == is_presence_expected From 36c5106a47b09bc75544cbb71b712f5963ba4b59 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 29 Dec 2022 06:36:09 +0200 Subject: [PATCH 07/25] chore: merge changes from master --- .github/workflows/precommit-check.yml | 38 +++++++++++++++++++++++++++ .universum.py | 3 ++- CHANGELOG.rst | 29 ++++++++++++++++++++ doc/jenkins.rst | 4 +-- tests/test_integration.py | 29 ++++++++++++++++++++ universum/__init__.py | 2 +- universum/modules/reporter.py | 8 ++++++ 7 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/precommit-check.yml diff --git a/.github/workflows/precommit-check.yml b/.github/workflows/precommit-check.yml new file mode 100644 index 00000000..bfa7d698 --- /dev/null +++ b/.github/workflows/precommit-check.yml @@ -0,0 +1,38 @@ +name: Universum check +on: push + +jobs: + universum_check: + name: Universum check + runs-on: ubuntu-latest + + steps: + - name: Setup python 3.8 + uses: actions/setup-python@v4 + with: + python-version: 3.8 + + - name: Install dependency + run: pip install universum[test] + + - name: Universum + run: + python -u -m universum + --vcs-type=git + --git-repo $GITHUB_SERVER_URL/$GITHUB_REPOSITORY + --git-refspec $GITHUB_REF_NAME + --no-archive + --no-diff + + - name: Publish Test Results + uses: EnricoMi/publish-unit-test-result-action@v1 + if: always() + with: + files: artifacts/*.xml + + - name: Collect artifacts + uses: actions/upload-artifact@v2 + if: ${{ always() }} + with: + name: artifacts + path: artifacts diff --git a/.universum.py b/.universum.py index 71dc3b24..e3c3eda8 100644 --- a/.universum.py +++ b/.universum.py @@ -20,12 +20,13 @@ def pip_install(module_name): configs = Variations([Step(name="Create virtual environment", command=[python, "-m", "venv", env_name]), Step(name="Update Docker images", command=run_virtual("make images")), - Step(name="Install Universum for tests", artifacts="junit_results.xml", + Step(name="Install Universum for tests", command=run_virtual(pip_install(".[test]"))), Step(name="Make", artifacts="doc/_build", command=run_virtual("make")), Step(name="Make tests", artifacts="htmlcov", command=run_virtual("export LANG=en_US.UTF-8; make test")), + Step(name="Collect test results", artifacts="junit_results.xml"), Step(name="Run static pylint", code_report=True, command=run_virtual(f"{python} -m universum.analyzers.pylint " diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d01c99ff..0ea21a2d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,35 @@ Change log ========== +0.19.14 (2022-11-14) +-------------------- + +New features +~~~~~~~~~~~~ + +* **main:** add option to return non-zero code if any step fails + + +0.19.13 (2022-10-10) +-------------------- + +New features +~~~~~~~~~~~~ + +* **automation:** add full support of GitHub Actions CI server (as alternative to Jenkins and TeamCity) +* **config:** add path to configuration file in :doc:`configuration_support `; + this might be very useful in non-CI mode when launched from directory other than project root +* **report:** status reports now include more useful info; previous short form can be enabled by + ``-rofs`` `command-line option `__ + +Bug fixes +~~~~~~~~~ + +* **config:** update error message if config is empty after step filtering +* **doc:** add explanation of filter usage in :ref:`non-CI mode description ` +* **analyzer:** update error message on missing input file + + 0.19.12 (2022-01-26) -------------------- diff --git a/doc/jenkins.rst b/doc/jenkins.rst index f22caa5f..d2784e4d 100644 --- a/doc/jenkins.rst +++ b/doc/jenkins.rst @@ -94,12 +94,12 @@ Here are the symptoms of domain names not resolving correctly: 1. Jenkins warnings when trying to save the updated settings 2. Client inability to access said pages (timeout error) -To set up domain name resolving, add following lines to server ``/ets/host`` file:: +To set up domain name resolving, add following lines to server ``/ets/hosts`` file:: 127.0.0.1
-And add the following lines to client ``/ets/host`` file:: +And add the following lines to client ``/ets/hosts`` file::
diff --git a/tests/test_integration.py b/tests/test_integration.py index dcb9529a..6b93202a 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -351,3 +351,32 @@ def test_abort(local_sources: LocalSources, tmpdir: py.path.local, terminate_typ time.sleep(5) process.send_signal(terminate_type) assert process.wait(5) == 3 + + +def test_exit_code(local_sources: LocalSources, tmpdir: py.path.local): + config = """ +from universum.configuration_support import Configuration + +configs = Configuration([dict(name="Unsuccessful step", command=["exit", "1"])]) +""" + config_file = tmpdir.join("configs.py") + config_file.write(config) + + with subprocess.Popen([python(), "-m", "universum", + "-o", "console", "-st", "local", "-vt", "none", + "-pr", str(tmpdir.join("project_root")), + "-ad", str(tmpdir.join("artifacts")), + "-fsd", str(local_sources.root_directory), + "-cfg", str(config_file)]) as process: + + assert process.wait() == 0 + + tmpdir.join("artifacts").remove(rec=True) + with subprocess.Popen([python(), "-m", "universum", "--fail-unsuccessful", + "-o", "console", "-st", "local", "-vt", "none", + "-pr", str(tmpdir.join("project_root")), + "-ad", str(tmpdir.join("artifacts")), + "-fsd", str(local_sources.root_directory), + "-cfg", str(config_file)]) as process: + assert process.wait() == 1 + diff --git a/universum/__init__.py b/universum/__init__.py index 701c4ef5..97c18b90 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.13" +__version__ = "0.19.15" diff --git a/universum/modules/reporter.py b/universum/modules/reporter.py index 0b8814ff..5c150563 100644 --- a/universum/modules/reporter.py +++ b/universum/modules/reporter.py @@ -6,6 +6,7 @@ from . import automation_server from .output import HasOutput from .structure_handler import HasStructure, Block +from ..lib.ci_exception import SilentAbortException from ..lib.gravity import Dependency from ..lib.utils import make_block @@ -53,6 +54,8 @@ def define_arguments(argument_parser): help="Include only the short list of failed steps to reporting comments") parser.add_argument("--report-no-vote", "-rnv", action="store_true", dest="no_vote", help="Do not vote up/down review depending on result") + parser.add_argument("--fail-unsuccessful", "-rfu", action="store_true", dest="fail_unsuccessful", + help="Return non-zero exit code if any step failed") def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -119,6 +122,8 @@ def report_build_result(self): if not self.observers: self.out.log("Nowhere to report. Skipping...") + if self.settings.fail_unsuccessful and not is_successful: + raise SilentAbortException(1) return if is_successful: @@ -149,6 +154,9 @@ def report_build_result(self): for observer in self.observers: observer.code_report_to_review(self.code_report_comments) + if self.settings.fail_unsuccessful and not is_successful: + raise SilentAbortException(1) + def _report_steps_recursively(self, block: Block, text: str, indent: str) -> Tuple[str, bool]: has_children: bool = bool(block.children) block_title: str = block.number + ' ' + block.name From d6c34961caed9e36ee7c76ba0da4e7349e2aeccd Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Mon, 3 Apr 2023 16:56:51 +0300 Subject: [PATCH 08/25] fix: pylint, mypy, doctest issues (#779) --- doc/configuring.rst | 2 +- pylintrc | 3 ++- tests/test_argument_check.py | 4 ++-- tests/test_conditional_steps.py | 8 ++++++-- tests/test_git_poll.py | 3 ++- tests/test_integration.py | 1 - tests/utils.py | 8 ++++---- universum/analyzers/utils.py | 2 +- universum/configuration_support.py | 2 +- universum/lib/utils.py | 4 ++-- universum/modules/artifact_collector.py | 6 ++---- universum/modules/output/terminal_based_output.py | 2 +- universum/modules/reporter.py | 10 +++++----- universum/modules/structure_handler.py | 3 +-- universum/modules/vcs/perforce_vcs.py | 12 ++++++------ universum/modules/vcs/vcs.py | 2 +- 16 files changed, 37 insertions(+), 35 deletions(-) diff --git a/doc/configuring.rst b/doc/configuring.rst index 05ec6ffe..ce0b5962 100644 --- a/doc/configuring.rst +++ b/doc/configuring.rst @@ -447,7 +447,7 @@ If the conditional step fails, the step from the ``if_failed`` parameter will be Configuration example: -.. testcode:: +.. code-block:: python from universum.configuration_support import Configuration, Step diff --git a/pylintrc b/pylintrc index 580f2ef8..7b39d854 100644 --- a/pylintrc +++ b/pylintrc @@ -20,7 +20,8 @@ disable = missing-docstring, no-member, unsupported-assignment-operation, super-init-not-called, - wildcard-import + wildcard-import, + use-dict-literal [BASIC] no-docstring-rgx = .* diff --git a/tests/test_argument_check.py b/tests/test_argument_check.py index 49344cf8..05974c80 100644 --- a/tests/test_argument_check.py +++ b/tests/test_argument_check.py @@ -1,4 +1,4 @@ -from typing import Union, List +from typing import Union, List, Optional import pytest from universum import __main__ @@ -101,7 +101,7 @@ def assert_incorrect_parameter(settings: ModuleNamespace, *args): def param(test_type: str, module: str, field: str, - vcs_type: Union[str, List[str]] = "*", error_match: str = None) -> None: + vcs_type: Union[str, List[str]] = "*", error_match: Optional[str] = None) -> None: if isinstance(vcs_type, list): for specific_vcs_type in vcs_type: diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 11273bed..af06ec05 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -1,3 +1,5 @@ +# pylint: disable = invalid-name + import re import os @@ -94,8 +96,10 @@ def build_step_command(files_to_create, exit_code): def write_config_file(tmpdir, conditional_steps_info): - true_branch_step = f"Step(**{str(conditional_steps_info.true_branch_step)})" if conditional_steps_info.true_branch_step else "None" - false_branch_step = f"Step(**{str(conditional_steps_info.false_branch_step)})" if conditional_steps_info.false_branch_step else "None" + true_branch_step = f"Step(**{str(conditional_steps_info.true_branch_step)})" \ + if conditional_steps_info.true_branch_step else "None" + false_branch_step = f"Step(**{str(conditional_steps_info.false_branch_step)})" \ + if conditional_steps_info.false_branch_step else "None" config_lines = [ "from universum.configuration_support import Configuration, Step", f"true_branch_step = {true_branch_step}", diff --git a/tests/test_git_poll.py b/tests/test_git_poll.py index 437014fe..f710b4e3 100644 --- a/tests/test_git_poll.py +++ b/tests/test_git_poll.py @@ -1,5 +1,6 @@ # pylint: disable = redefined-outer-name +from typing import Optional import py import pytest @@ -13,7 +14,7 @@ def git_poll_environment(git_client: GitClient, tmpdir: py.path.local): yield GitTestEnvironment(git_client, tmpdir, test_type="poll") -def make_branch_with_changes(git_server: GitServer, branch_name: str, commits_number: int, branch_from: str = None): +def make_branch_with_changes(git_server: GitServer, branch_name: str, commits_number: int, branch_from: Optional[str] = None): """ Creates a branch from the current or specified (by name) and adds passed commits number. diff --git a/tests/test_integration.py b/tests/test_integration.py index 6b93202a..d7a3749a 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -379,4 +379,3 @@ def test_exit_code(local_sources: LocalSources, tmpdir: py.path.local): "-fsd", str(local_sources.root_directory), "-cfg", str(config_file)]) as process: assert process.wait() == 1 - diff --git a/tests/utils.py b/tests/utils.py index 1fcc08ab..e05c8a01 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -109,20 +109,20 @@ def create_empty_settings(test_type: str) -> ModuleNamespace: class BaseVcsClient: - def __init__(self): + def __init__(self) -> None: self.root_directory: py.path.local self.repo_file: py.path.local def get_last_change(self): pass - def file_present(self, file_path: str) -> bool: + def file_present(self, file_path: str) -> bool: # type: ignore pass - def text_in_file(self, text: str, file_path: str) -> bool: + def text_in_file(self, text: str, file_path: str) -> bool: # type: ignore pass - def make_a_change(self) -> str: + def make_a_change(self) -> str: # type: ignore pass diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index 76e70175..a6bfd452 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -137,7 +137,7 @@ def add_python_version_argument(parser: argparse.ArgumentParser) -> None: "'python3.7 -m pylint <...>'") -def report_to_file(issues: List[ReportData], json_file: str = None) -> None: +def report_to_file(issues: List[ReportData], json_file: Optional[str] = None) -> None: issues_json = json.dumps(issues, indent=4) if json_file: with open(json_file, "w", encoding="utf-8") as f: diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 91bbaf93..482285c6 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -686,7 +686,7 @@ def dump(self, produce_string_command: bool = True) -> str: return result def filter(self, checker: Callable[[Step], bool], - parent: Step = None) -> 'Configuration': + parent: Optional[Step] = None) -> 'Configuration': """ This function is supposed to be called from main script, not configuration file. It uses provided `checker` to find all the configurations that pass the check, diff --git a/universum/lib/utils.py b/universum/lib/utils.py index 02154d7f..b924a6e9 100644 --- a/universum/lib/utils.py +++ b/universum/lib/utils.py @@ -104,10 +104,10 @@ def format_traceback(exc: Exception, trace: Optional[TracebackType]) -> str: return tb_text -def catch_exception(exception_name: str, ignore_if: str = None) -> DecoratorT: +def catch_exception(exception_name: str, ignore_if: Optional[str] = None) -> DecoratorT: def decorated_function(function): def function_to_run(*args, **kwargs): - result: ReturnT = None + result: ReturnT = None # type: ignore try: result = function(*args, **kwargs) return result diff --git a/universum/modules/artifact_collector.py b/universum/modules/artifact_collector.py index cfdbcdf5..d0f1312a 100644 --- a/universum/modules/artifact_collector.py +++ b/universum/modules/artifact_collector.py @@ -1,6 +1,4 @@ import codecs -import distutils -from distutils import dir_util, errors import os import shutil import zipfile @@ -228,11 +226,11 @@ def move_artifact(self, path, is_report=False): # Single file archiving is not implemented at the moment pass try: - distutils.dir_util.copy_tree(matching_path, destination) + shutil.copytree(matching_path, destination) if is_report: text = "'" + artifact_name + "' is not a file and cannot be reported as an artifact" self.out.log(text) - except distutils.errors.DistutilsFileError: + except NotADirectoryError: shutil.copyfile(matching_path, destination) if is_report: artifact_path = self.automation_server.artifact_path(self.artifact_dir, artifact_name) diff --git a/universum/modules/output/terminal_based_output.py b/universum/modules/output/terminal_based_output.py index f4004d01..980340c4 100644 --- a/universum/modules/output/terminal_based_output.py +++ b/universum/modules/output/terminal_based_output.py @@ -23,7 +23,7 @@ class TerminalBasedOutput(BaseOutput): def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.block_level = 0 - self.unicode_acceptable = (locale.getpreferredencoding() == "UTF-8") + self.unicode_acceptable = (locale.getpreferredencoding() == "UTF-8") # pylint: disable = superfluous-parens @staticmethod def _stdout(*args, **kwargs) -> None: diff --git a/universum/modules/reporter.py b/universum/modules/reporter.py index 5c150563..0d3c95c2 100644 --- a/universum/modules/reporter.py +++ b/universum/modules/reporter.py @@ -57,12 +57,12 @@ def define_arguments(argument_parser): parser.add_argument("--fail-unsuccessful", "-rfu", action="store_true", dest="fail_unsuccessful", help="Return non-zero exit code if any step failed") - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self.observers = [] - self.report_initialized = False - self.blocks_to_report = [] - self.artifacts_to_report = [] + self.observers: List = [] + self.report_initialized: bool = False + self.blocks_to_report: List = [] + self.artifacts_to_report: List = [] self.code_report_comments: Dict[str, List[ReportMessage]] = defaultdict(list) self.automation_server = self.automation_server_factory() diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index 8064a22a..8a81cffb 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -254,8 +254,7 @@ def execute_steps_recursively(self, parent: Step, children=Configuration([step_to_execute]), step_executor=step_executor, skip_execution=False) - else: # conditional step should be always successful - current_step_failed = False + current_step_failed = False # conditional step should be always successful else: if merged_item.finish_background and self.active_background_steps: self.out.log("All ongoing background steps should be finished before next step execution") diff --git a/universum/modules/vcs/perforce_vcs.py b/universum/modules/vcs/perforce_vcs.py index 8974b3d7..e0fc8618 100644 --- a/universum/modules/vcs/perforce_vcs.py +++ b/universum/modules/vcs/perforce_vcs.py @@ -299,7 +299,7 @@ def define_arguments(argument_parser): help="**Revert all vcs within '--p4-client' and delete the workspace.** " "Mandatory for CI environment, otherwise use with caution") - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.check_required_option("client", """ @@ -318,11 +318,11 @@ def __init__(self, *args, **kwargs): self.client_name = self.settings.client self.client_root = self.settings.project_root - self.sync_cls = [] - self.shelve_cls = [] - self.depots = [] - self.client_view = [] - self.mappings_dict = {} + self.sync_cls: List = [] + self.shelve_cls: List = [] + self.depots: List = [] + self.client_view: List = [] + self.mappings_dict: Dict = {} self.unshelved_files: List[Dict[str, str]] = [] self.diff_in_files: List[Tuple[Optional[str], Optional[str], Optional[str]]] = [] diff --git a/universum/modules/vcs/vcs.py b/universum/modules/vcs/vcs.py index cb31fd08..9884bd6f 100644 --- a/universum/modules/vcs/vcs.py +++ b/universum/modules/vcs/vcs.py @@ -20,7 +20,7 @@ ] -def create_vcs(class_type: str = None) -> Type[ProjectDirectory]: +def create_vcs(class_type: Optional[str] = None) -> Type[ProjectDirectory]: driver_factory_class: Union[ Dict[str, Type[base_vcs.BasePollVcs]], Dict[str, Type[base_vcs.BaseSubmitVcs]], From 17310763271c2b3e4fe066dec3bb110d8173b0a3 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Tue, 4 Apr 2023 09:39:29 +0300 Subject: [PATCH 09/25] fix: merge errors (#780) --- universum/modules/reporter.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/universum/modules/reporter.py b/universum/modules/reporter.py index 26511075..7819bfab 100644 --- a/universum/modules/reporter.py +++ b/universum/modules/reporter.py @@ -53,8 +53,6 @@ def define_arguments(argument_parser): help="Include only the short list of failed steps to reporting comments") parser.add_argument("--report-no-vote", "-rnv", action="store_true", dest="no_vote", help="Do not vote up/down review depending on result") - parser.add_argument("--fail-unsuccessful", "-rfu", action="store_true", dest="fail_unsuccessful", - help="Return non-zero exit code if any step failed") def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) From 81d40f990701b58d7f8e2786de73fc48178af0dc Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 20 Apr 2023 16:39:07 +0300 Subject: [PATCH 10/25] test: reformat test_conditional_steps.py (#790) --- tests/test_conditional_steps.py | 290 ++++++++++++++++---------------- 1 file changed, 146 insertions(+), 144 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index af06ec05..b94308b9 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -1,171 +1,173 @@ -# pylint: disable = invalid-name +# pylint: disable = invalid-name, redefined-outer-name import re -import os +import pathlib +from typing import Generator, AnyStr, List, Optional + +import pytest -from universum import __main__ from universum.configuration_support import Step +from .utils import LocalTestEnvironment + class StepsInfo: - conditional_step = None - true_branch_step = None - false_branch_step = None - is_conditional_step_passed = False + conditional_step: Step + true_branch_step: Optional[Step] + false_branch_step: Optional[Step] + is_conditional_step_passed: bool + + +class ConditionalStepsTestEnv(LocalTestEnvironment): + + def __init__(self, tmp_path: pathlib.Path, capsys: pytest.CaptureFixture) -> None: + super().__init__(tmp_path, "main") + self.capsys = capsys + + def build_conditional_steps_info(self, is_conditional_step_passed: bool) -> StepsInfo: + steps_info: StepsInfo = StepsInfo() + + steps_info.conditional_step = self._build_conditional_step(is_conditional_step_passed) + steps_info.true_branch_step = self._build_true_branch_step() + steps_info.false_branch_step = self._build_false_branch_step() + steps_info.is_conditional_step_passed = is_conditional_step_passed + + return steps_info + + def check_conditional_step(self, steps_info: StepsInfo) -> None: + self._write_config_file(steps_info) + self.run() + + captured: pytest.CaptureResult[AnyStr] = self.capsys.readouterr() + conditional_succeeded_regexp: str = r"\] conditional.*Success.*\| 5\.2" + assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) + + self._check_conditional_step_artifacts_present(steps_info) + self._check_executed_step_artifacts_present(steps_info) + self._check_not_executed_step_artifacts_absent(steps_info) + + def _build_conditional_step(self, is_conditional_step_passed: bool) -> Step: + conditional_step_name: str = "conditional" + conditional_step_artifact: str = f"{conditional_step_name}_artifact" + conditional_step_report_artifact: str = f"{conditional_step_name}_report_artifact" + conditional_step_exit_code: int = 0 if is_conditional_step_passed else 1 + return Step(name=conditional_step_name, + command=self._build_step_command( + files_to_create=[conditional_step_artifact, conditional_step_report_artifact], + exit_code=conditional_step_exit_code), + artifacts=conditional_step_artifact, + report_artifacts=conditional_step_report_artifact) + + def _build_true_branch_step(self) -> Step: + true_branch_step_name: str = "true_branch" + true_branch_step_artifact: str = f"{true_branch_step_name}_artifact" + true_branch_step_report_artifact: str = f"{true_branch_step_name}_report_artifact" + return Step(name=true_branch_step_name, + command=self._build_step_command( + files_to_create=[true_branch_step_artifact, true_branch_step_report_artifact], + exit_code=0), + artifacts=true_branch_step_artifact, + report_artifacts=true_branch_step_report_artifact) + + def _build_false_branch_step(self) -> Step: + false_branch_step_name: str = "false_branch" + false_branch_step_artifact: str = f"{false_branch_step_name}_artifact" + false_branch_step_report_artifact: str = f"{false_branch_step_name}_report_artifact" + return Step(name=false_branch_step_name, + command=self._build_step_command( + files_to_create=[false_branch_step_artifact, false_branch_step_report_artifact], + exit_code=0), + artifacts=false_branch_step_artifact, + report_artifacts=false_branch_step_report_artifact) + + @staticmethod + def _build_step_command(files_to_create, exit_code) -> List[str]: + commands: List[str] = [] + for f in files_to_create: + commands.append(f"touch {f}") + commands.append(f"exit {exit_code}") + return ["bash", "-c", ";".join(commands)] + + def _write_config_file(self, steps_info) -> None: + true_branch_step: str = f"Step(**{str(steps_info.true_branch_step)})" \ + if steps_info.true_branch_step else "None" + false_branch_step: str = f"Step(**{str(steps_info.false_branch_step)})" \ + if steps_info.false_branch_step else "None" + config_lines: List[str] = [ + "from universum.configuration_support import Configuration, Step", + f"true_branch_step = {true_branch_step}", + f"false_branch_step = {false_branch_step}", + f"conditional_step = Step(**{str(steps_info.conditional_step)})", + + # `true/false_branch_steps` should be Python objects from this script + "conditional_step.is_conditional = True", + "conditional_step.if_succeeded = true_branch_step", + "conditional_step.if_failed = false_branch_step", + + "configs = Configuration([conditional_step])" + ] + config: str = "\n".join(config_lines) + + self.configs_file.write_text(config, "utf-8") + + def _check_conditional_step_artifacts_present(self, steps_info: StepsInfo) -> None: + conditional_step: Step = steps_info.conditional_step + self._check_artifacts_presence(conditional_step, is_presence_expected=True) + + def _check_executed_step_artifacts_present(self, steps_info: StepsInfo) -> None: + executed_step: Optional[Step] = steps_info.true_branch_step if steps_info.is_conditional_step_passed \ + else steps_info.false_branch_step + self._check_artifacts_presence(executed_step, is_presence_expected=True) + + def _check_not_executed_step_artifacts_absent(self, steps_info: StepsInfo) -> None: + not_executed_step: Optional[Step] = steps_info.false_branch_step if steps_info.is_conditional_step_passed \ + else steps_info.true_branch_step + self._check_artifacts_presence(not_executed_step, is_presence_expected=False) + + def _check_artifacts_presence(self, step: Optional[Step], is_presence_expected: bool): + if not step: # branch step can be not set + return + for artifact in [step.artifacts, step.report_artifacts]: + artifact_path: pathlib.Path = self.artifact_dir / artifact + assert artifact_path.exists() == is_presence_expected + +@pytest.fixture() +def test_env(tmp_path: pathlib.Path, capsys: pytest.CaptureFixture) -> Generator[ConditionalStepsTestEnv, None, None]: + yield ConditionalStepsTestEnv(tmp_path, capsys) -def test_conditional_true_branch(tmpdir, capsys): - steps_info = get_conditional_steps_info(is_conditional_step_passed=True) - check_conditional_step(tmpdir, capsys, steps_info) +def test_conditional_true_branch(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) + test_env.check_conditional_step(steps_info) -def test_conditional_false_branch(tmpdir, capsys): - steps_info = get_conditional_steps_info(is_conditional_step_passed=False) - check_conditional_step(tmpdir, capsys, steps_info) + +def test_conditional_false_branch(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) + test_env.check_conditional_step(steps_info) # https://github.com/Samsung/Universum/issues/744 # Artifact will be collected only from the second executed step, the first one will be overwritten # Skipping checking file content to not overload tests with additional logic for incorrect behaviour check -def test_same_artifact(tmpdir, capsys): - steps_info = get_conditional_steps_info(is_conditional_step_passed=True) +def test_same_artifact(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) + assert steps_info.true_branch_step steps_info.true_branch_step.command = steps_info.conditional_step.command steps_info.true_branch_step.artifacts = steps_info.conditional_step.artifacts steps_info.true_branch_step.report_artifacts = steps_info.conditional_step.report_artifacts - check_conditional_step(tmpdir, capsys, steps_info) + test_env.check_conditional_step(steps_info) -def test_true_branch_chosen_but_absent(tmpdir, capsys): - steps_info = get_conditional_steps_info(is_conditional_step_passed=True) +def test_true_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) steps_info.true_branch_step = None - check_conditional_step(tmpdir, capsys, steps_info) + test_env.check_conditional_step(steps_info) -def test_false_branch_chosen_but_absent(tmpdir, capsys): - steps_info = get_conditional_steps_info(is_conditional_step_passed=False) +def test_false_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) steps_info.false_branch_step = None - check_conditional_step(tmpdir, capsys, steps_info) - - -def get_conditional_steps_info(is_conditional_step_passed): - steps_info = StepsInfo() - - conditional_step_name = "conditional" - conditional_step_artifact = f"{conditional_step_name}_artifact" - conditional_step_report_artifact = f"{conditional_step_name}_report_artifact" - conditional_step_exit_code = 0 if is_conditional_step_passed else 1 - steps_info.conditional_step = Step( - name=conditional_step_name, - command=build_step_command(files_to_create=[conditional_step_artifact, conditional_step_report_artifact], - exit_code=conditional_step_exit_code), - artifacts=conditional_step_artifact, - report_artifacts=conditional_step_report_artifact) - steps_info.is_conditional_step_passed = is_conditional_step_passed - - true_branch_step_name = "true_branch" - true_branch_step_artifact = f"{true_branch_step_name}_artifact" - true_branch_step_report_artifact = f"{true_branch_step_name}_report_artifact" - steps_info.true_branch_step = Step( - name=true_branch_step_name, - command=build_step_command(files_to_create=[true_branch_step_artifact, true_branch_step_report_artifact], - exit_code=0), - artifacts=true_branch_step_artifact, - report_artifacts=true_branch_step_report_artifact) - - false_branch_step_name = "false_branch" - false_branch_step_artifact = f"{false_branch_step_name}_artifact" - false_branch_step_report_artifact = f"{false_branch_step_name}_report_artifact" - steps_info.false_branch_step = Step( - name=false_branch_step_name, - command=build_step_command(files_to_create=[false_branch_step_artifact, false_branch_step_report_artifact], - exit_code=0), - artifacts=false_branch_step_artifact, - report_artifacts=false_branch_step_report_artifact) - - return steps_info - - -def build_step_command(files_to_create, exit_code): - commands = [] - for f in files_to_create: - commands.append(f"touch {f}") - commands.append(f"exit {exit_code}") - return ["bash", "-c", ";".join(commands)] - - -def write_config_file(tmpdir, conditional_steps_info): - true_branch_step = f"Step(**{str(conditional_steps_info.true_branch_step)})" \ - if conditional_steps_info.true_branch_step else "None" - false_branch_step = f"Step(**{str(conditional_steps_info.false_branch_step)})" \ - if conditional_steps_info.false_branch_step else "None" - config_lines = [ - "from universum.configuration_support import Configuration, Step", - f"true_branch_step = {true_branch_step}", - f"false_branch_step = {false_branch_step}", - f"conditional_step = Step(**{str(conditional_steps_info.conditional_step)})", - - # `true/false_branch_steps` should be Python objects from this script - "conditional_step.is_conditional = True", - "conditional_step.if_succeeded = true_branch_step", - "conditional_step.if_failed = false_branch_step", - - "configs = Configuration([conditional_step])" - ] - config = "\n".join(config_lines) - - config_file = tmpdir.join("configs.py") - config_file.write_text(config, "utf-8") - - return config_file - - -def check_conditional_step(tmpdir, capsys, steps_info): - config_file = write_config_file(tmpdir, steps_info) - - artifacts_dir = tmpdir.join("artifacts") - params = ["-vt", "none", - "-fsd", str(tmpdir), - "-ad", str(artifacts_dir), - "--clean-build", - "-o", "console"] - params.extend(["-cfg", str(config_file)]) - - return_code = __main__.main(params) - assert return_code == 0 - - captured = capsys.readouterr() - conditional_succeeded_regexp = r"\] conditional.*Success.*\| 5\.2" - assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) - - artifacts_checker = ConditionalStepsArtifactChecker(artifacts_dir, steps_info) - artifacts_checker.check_conditional_step_artifacts_present() - artifacts_checker.check_executed_step_artifacts_present() - artifacts_checker.check_not_executed_step_artifacts_absent() - - -class ConditionalStepsArtifactChecker: - - def __init__(self, artifacts_dir, steps_info): - self._artifacts_dir = artifacts_dir - self._conditional_step = steps_info.conditional_step - self._executed_step = steps_info.true_branch_step if steps_info.is_conditional_step_passed \ - else steps_info.false_branch_step - self._not_executed_step = steps_info.false_branch_step if steps_info.is_conditional_step_passed \ - else steps_info.true_branch_step - - def check_conditional_step_artifacts_present(self): - self._check_artifacts_presence(self._conditional_step, is_presence_expected=True) - - def check_executed_step_artifacts_present(self): - self._check_artifacts_presence(self._executed_step, is_presence_expected=True) - - def check_not_executed_step_artifacts_absent(self): - self._check_artifacts_presence(self._not_executed_step, is_presence_expected=False) - - def _check_artifacts_presence(self, step, is_presence_expected): - if not step: # branch step can be not set - return - for artifact in [step.artifacts, step.report_artifacts]: - assert os.path.exists(os.path.join(self._artifacts_dir, artifact)) == is_presence_expected + test_env.check_conditional_step(steps_info) From 9f4cb4e1ae5991399ab7e06d21d3d6b3d619bbd3 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Mon, 24 Apr 2023 15:27:44 +0300 Subject: [PATCH 11/25] test(artifact_collector): artifacts preprocessing for conditional steps (#791) --- tests/test_conditional_steps.py | 38 +++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index b94308b9..4db0308b 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -34,7 +34,11 @@ def build_conditional_steps_info(self, is_conditional_step_passed: bool) -> Step return steps_info - def check_conditional_step(self, steps_info: StepsInfo) -> None: + def create_step_artifact(self, step: Step) -> None: + artifact_path: pathlib.Path = self.src_dir / step.artifacts + artifact_path.touch() + + def check_success(self, steps_info: StepsInfo) -> None: self._write_config_file(steps_info) self.run() @@ -46,6 +50,10 @@ def check_conditional_step(self, steps_info: StepsInfo) -> None: self._check_executed_step_artifacts_present(steps_info) self._check_not_executed_step_artifacts_absent(steps_info) + def check_fail(self, steps_info: StepsInfo) -> None: + self._write_config_file(steps_info) + self.run(expect_failure=True) + def _build_conditional_step(self, is_conditional_step_passed: bool) -> Step: conditional_step_name: str = "conditional" conditional_step_artifact: str = f"{conditional_step_name}_artifact" @@ -139,12 +147,12 @@ def test_env(tmp_path: pathlib.Path, capsys: pytest.CaptureFixture) -> Generator def test_conditional_true_branch(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) - test_env.check_conditional_step(steps_info) + test_env.check_success(steps_info) def test_conditional_false_branch(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) - test_env.check_conditional_step(steps_info) + test_env.check_success(steps_info) # https://github.com/Samsung/Universum/issues/744 @@ -158,16 +166,34 @@ def test_same_artifact(test_env: ConditionalStepsTestEnv) -> None: steps_info.true_branch_step.artifacts = steps_info.conditional_step.artifacts steps_info.true_branch_step.report_artifacts = steps_info.conditional_step.report_artifacts - test_env.check_conditional_step(steps_info) + test_env.check_success(steps_info) def test_true_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) steps_info.true_branch_step = None - test_env.check_conditional_step(steps_info) + test_env.check_success(steps_info) def test_false_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) steps_info.false_branch_step = None - test_env.check_conditional_step(steps_info) + test_env.check_success(steps_info) + + +@pytest.mark.parametrize("is_branch_step", [True, False]) +@pytest.mark.parametrize("prebuild_clean", [True, False]) +def test_prebuild_clean(test_env: ConditionalStepsTestEnv, + is_branch_step: bool, + prebuild_clean: bool) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) + + assert steps_info.true_branch_step + step: Step = steps_info.true_branch_step if is_branch_step else steps_info.conditional_step + step.artifact_prebuild_clean = prebuild_clean + test_env.create_step_artifact(step) + + if prebuild_clean: + test_env.check_success(steps_info) + else: + test_env.check_fail(steps_info) From 7ad35b0bc4a6a986faf13c21546b9893363bf1ec Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Mon, 1 May 2023 09:22:40 +0300 Subject: [PATCH 12/25] feat(artifact_collector): preprocess report artifacts for conditional steps (#794) --- tests/test_conditional_steps.py | 11 ++++--- universum/modules/artifact_collector.py | 42 +++++++++++++------------ 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 4db0308b..a7316370 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -34,8 +34,9 @@ def build_conditional_steps_info(self, is_conditional_step_passed: bool) -> Step return steps_info - def create_step_artifact(self, step: Step) -> None: - artifact_path: pathlib.Path = self.src_dir / step.artifacts + def create_step_artifact(self, step: Step, is_report_artifact: bool = False) -> None: + file_name: str = step.report_artifacts if is_report_artifact else step.artifacts + artifact_path: pathlib.Path = self.src_dir / file_name artifact_path.touch() def check_success(self, steps_info: StepsInfo) -> None: @@ -183,15 +184,17 @@ def test_false_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> No @pytest.mark.parametrize("is_branch_step", [True, False]) @pytest.mark.parametrize("prebuild_clean", [True, False]) +@pytest.mark.parametrize("is_report_artifact", [True, False]) def test_prebuild_clean(test_env: ConditionalStepsTestEnv, is_branch_step: bool, - prebuild_clean: bool) -> None: + prebuild_clean: bool, + is_report_artifact: bool) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) assert steps_info.true_branch_step step: Step = steps_info.true_branch_step if is_branch_step else steps_info.conditional_step step.artifact_prebuild_clean = prebuild_clean - test_env.create_step_artifact(step) + test_env.create_step_artifact(step, is_report_artifact) if prebuild_clean: test_env.check_success(steps_info) diff --git a/universum/modules/artifact_collector.py b/universum/modules/artifact_collector.py index ecfd744e..4fdc36b9 100644 --- a/universum/modules/artifact_collector.py +++ b/universum/modules/artifact_collector.py @@ -164,11 +164,10 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin for configuration in project_configs.all(): if configuration.artifacts: artifact_list.append(self.get_config_artifact(configuration)) + if configuration.report_artifacts: + artifact_list.append(self.get_config_artifact(configuration, is_report_artifact=True)) if configuration.is_conditional: artifact_list.extend(self.get_conditional_step_branches_artifacts(configuration)) - if configuration.report_artifacts: - path: str = utils.parse_path(configuration.report_artifacts, self.settings.project_root) - report_artifact_list.append(dict(path=path, clean=configuration.artifact_prebuild_clean)) if artifact_list: name = "Setting and preprocessing artifacts according to configs" @@ -181,23 +180,26 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin self.preprocess_artifact_list(report_artifact_list, ignore_existing_artifacts) def get_conditional_step_branches_artifacts(self, step: Step) -> List[ArtifactInfo]: - succeeded_config_artifact: Optional[ArtifactInfo] = self.get_config_artifact_if_exists(step.if_succeeded) - failed_config_artifact: Optional[ArtifactInfo] = self.get_config_artifact_if_exists(step.if_failed) - - artifacts = [] - if succeeded_config_artifact: - artifacts.append(succeeded_config_artifact) - if failed_config_artifact: - artifacts.append(failed_config_artifact) - return artifacts - - def get_config_artifact_if_exists(self, step: Step) -> Optional[ArtifactInfo]: - if step and step.artifacts: - return self.get_config_artifact(step) - return None - - def get_config_artifact(self, step: Step) -> ArtifactInfo: - path = utils.parse_path(step.artifacts, self.settings.project_root) + artifacts: List[Optional[ArtifactInfo]] = [ + self.get_config_artifact_if_exists(step.if_succeeded), + self.get_config_artifact_if_exists(step.if_failed), + self.get_config_artifact_if_exists(step.if_succeeded, is_report_artifact=True), + self.get_config_artifact_if_exists(step.if_failed, is_report_artifact=True) + ] + defined_artifacts: List[ArtifactInfo] = [artifact for artifact in artifacts if artifact] + return defined_artifacts + + def get_config_artifact_if_exists(self, step: Step, is_report_artifact: bool = False) -> Optional[ArtifactInfo]: + if not step: + return None + artifact: str = step.report_artifacts if is_report_artifact else step.artifacts + if not artifact: + return None + return self.get_config_artifact(step, is_report_artifact) + + def get_config_artifact(self, step: Step, is_report_artifact: bool = False) -> ArtifactInfo: + artifact: str = step.report_artifacts if is_report_artifact else step.artifacts + path: str = utils.parse_path(artifact, self.settings.project_root) return dict(path=path, clean=step.artifact_prebuild_clean) def move_artifact(self, path, is_report=False): From 020d83d910f088d2b094759a60d627d2d63aa8c6 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 4 May 2023 12:45:20 +0300 Subject: [PATCH 13/25] fix(artifact_collector): pre-process artifacts and report artifacts in a single step (#795) --- universum/modules/artifact_collector.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/universum/modules/artifact_collector.py b/universum/modules/artifact_collector.py index 4fdc36b9..c6748d23 100644 --- a/universum/modules/artifact_collector.py +++ b/universum/modules/artifact_collector.py @@ -160,7 +160,6 @@ def preprocess_artifact_list(self, artifact_list, ignore_already_existing=False) def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existing_artifacts: bool = False) -> None: self.html_output.artifact_dir_ready = True artifact_list: List[ArtifactInfo] = [] - report_artifact_list: List[ArtifactInfo] = [] for configuration in project_configs.all(): if configuration.artifacts: artifact_list.append(self.get_config_artifact(configuration)) @@ -174,11 +173,6 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin with self.structure.block(block_name=name, pass_errors=True): 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.preprocess_artifact_list(report_artifact_list, ignore_existing_artifacts) - def get_conditional_step_branches_artifacts(self, step: Step) -> List[ArtifactInfo]: artifacts: List[Optional[ArtifactInfo]] = [ self.get_config_artifact_if_exists(step.if_succeeded), From fcfdf6be550bfc5866afec59ecd0743f372e239a Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 18 May 2023 07:13:40 +0300 Subject: [PATCH 14/25] chore: merge changes from master (#803) --- .github/workflows/telegram-bot.yml | 8 +- CHANGELOG.rst | 17 +++ doc/code_report.rst | 42 ++++++- doc/conf.py | 2 +- pylintrc | 5 +- setup.py | 7 +- tests/deployment_utils.py | 3 +- tests/test_code_report.py | 63 +++++++--- tests/test_nonci.py | 2 +- universum/__init__.py | 2 +- universum/analyzers/clang_format.py | 67 +++++++++++ universum/analyzers/diff_utils.py | 133 +++++++++++++++++++++ universum/analyzers/mypy.py | 3 +- universum/analyzers/pylint.py | 3 +- universum/analyzers/uncrustify.py | 132 ++------------------ universum/analyzers/utils.py | 28 ++++- universum/configuration_support.py | 10 +- universum/modules/code_report_collector.py | 3 - universum/modules/vcs/swarm.py | 2 +- 19 files changed, 368 insertions(+), 164 deletions(-) create mode 100755 universum/analyzers/clang_format.py create mode 100644 universum/analyzers/diff_utils.py diff --git a/.github/workflows/telegram-bot.yml b/.github/workflows/telegram-bot.yml index 6d623abc..c319b739 100644 --- a/.github/workflows/telegram-bot.yml +++ b/.github/workflows/telegram-bot.yml @@ -23,6 +23,7 @@ jobs: PR_MERGED: ${{ github.event.pull_request.merged_by.login }} REVIEW_STATE: ${{ github.event.review.state }} REVIEW_AUTHOR: ${{ github.event.review.user.login }} + REVIEW_COMMENT: ${{ github.event.review.body }} COMMENT_AUTHOR: ${{ github.event.comment.user.login }} COMMENT_URL: ${{ github.event.comment.html_url }} COMMENT_BODY: ${{ github.event.comment.body }} @@ -51,7 +52,10 @@ jobs: elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} requested changes for PR#${{ env.PR_NUMBER }}"` - elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" != "changes_requested" ]]; then + elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "commented" ]]; then + ESCAPED_TEXT=`echo -e "${{ env.REVIEW_COMMENT }}"| sed 's/\&/\&/g' | sed 's//\>/g'` + TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} posted the following comment to PR#${{ env.PR_NUMBER }}:\n$ESCAPED_TEXT"` + elif [[ ! -z "${{ github.event.review }}" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} ${{ env.REVIEW_STATE }} PR#${{ env.PR_NUMBER }}"` elif [[ -z "${{ github.event.review }}" && "${{ github.event.action }}" == "submitted" ]]; then TEXT=`echo -e "Due to GitHub Actions bug we cannot identify, who approved PR#${{ env.PR_NUMBER }}"` @@ -63,7 +67,7 @@ jobs: fi if [[ ! -z $TEXT ]]; then - curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" \ + curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" --data-urlencode "disable_web_page_preview=True" \ --data-urlencode "text=$TEXT" --data-urlencode "parse_mode=HTML" $URL fi diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0ea21a2d..0678da9b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,23 @@ Change log ========== +0.19.15 (2023-05-10) +-------------------- + +New features +~~~~~~~~~~~~ + +* **analyzer:** code report based on clang-format + +Bug fixes +~~~~~~~~~ + +* **config:** fixed returning "None" instead of "False" for Step custom keys set to "False" +* **artifact:** remove redundant Static_analysis_report.json from artifacts +* **analyzer:** incorrect program name in help +* **report:** set exit code even if reporting crashes + + 0.19.14 (2022-11-14) -------------------- diff --git a/doc/code_report.rst b/doc/code_report.rst index a9ca4b86..48317dcf 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -6,6 +6,7 @@ The following analysing modules (analysers) are currently added to Universum: * `pylint`_ * `mypy`_ * `uncrustify`_ + * `clang-format`_ Analysers are separate scripts, fully compatible with Universum. It is possible to use them as independent Python modules. @@ -62,7 +63,6 @@ Pylint .. argparse:: :ref: universum.analyzers.pylint.pylint_argument_parser - :prog: {python} -m universum.analyzers.pylint Config example for ``universum.analyzers.pylint``: @@ -99,7 +99,6 @@ Mypy .. argparse:: :ref: universum.analyzers.mypy.mypy_argument_parser - :prog: {python} -m universum.analyzers.mypy Config example for ``universum.analyzers.mypy``: @@ -136,7 +135,6 @@ Uncrustify .. argparse:: :ref: universum.analyzers.uncrustify.uncrustify_argument_parser - :prog: {python} -m universum.analyzers.uncrustify :nodefault: Config example for ``universum.analyzers.uncrustify``: @@ -146,7 +144,7 @@ Config example for ``universum.analyzers.uncrustify``: from universum.configuration_support import Configuration, Step configs = Configuration([Step(name="uncrustify", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp", + "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp/*.c", "--cfg-file", "file_name.cfg", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "uncrustify" ])]) @@ -164,4 +162,38 @@ will produce this list of configurations: .. testoutput:: $ ./.universum.py - [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] + [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp/*.c --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] + +Clang-format +------------ + +.. argparse:: + :ref: universum.analyzers.clang_format.clang_format_argument_parser + :nodefault: + +Config example for ``universum.analyzers.clang_format``: + +.. testcode:: + + from universum.configuration_support import Configuration, Step + + configs = Configuration([Step(name="clang-format", code_report=True, command=[ + "{python}", "-m", "universum.analyzers.clang-format", "--files", "/home/user/workspace/temp/*.c", + "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "clang-format", "-e", "clang-format-15", + ])]) + + if __name__ == '__main__': + print(configs.dump()) + +will produce this list of configurations: + +.. testcode:: + :hide: + + print("$ ./.universum.py") + print(configs.dump()) + +.. testoutput:: + + $ ./.universum.py + [{'name': 'clang-format', 'code_report': True, 'command': '{python} -m universum.analyzers.clang-format --files /home/user/workspace/temp/*.c --result-file ${CODE_REPORT_FILE} --output-directory clang-format -e clang-format-15'}] diff --git a/doc/conf.py b/doc/conf.py index 229cdb94..d25e3408 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -50,7 +50,7 @@ # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = None +language = 'en' # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. diff --git a/pylintrc b/pylintrc index 7b39d854..ee326fca 100644 --- a/pylintrc +++ b/pylintrc @@ -37,4 +37,7 @@ max-parents = 10 max-line-length = 130 [SIMILARITIES] -ignore-imports=yes \ No newline at end of file +ignore-imports=yes + +[TYPECHECK] +signature-mutators=universum.analyzers.utils.analyzer diff --git a/setup.py b/setup.py index 642aa26c..e5dc59d4 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,8 @@ def readme(): 'sh', 'lxml', 'typing-extensions', - 'ansi2html' + 'ansi2html', + 'pyyaml==6.0' ], extras_require={ 'p4': [p4], @@ -56,7 +57,9 @@ def readme(): 'coverage', 'mypy', 'types-requests', - 'selenium==3.141' + 'selenium==3.141', + 'urllib3==1.26.15', # This is required for selenium-3.141 to work correctly + 'types-PyYAML==6.0' ] }, package_data={'': ['*.css', '*.js']} diff --git a/tests/deployment_utils.py b/tests/deployment_utils.py index 8d080315..32a67194 100644 --- a/tests/deployment_utils.py +++ b/tests/deployment_utils.py @@ -42,7 +42,8 @@ def add_bind_dirs(self, directories): if self._container: self.request.raiseerror("Container is already running, no dirs can be bound!") for directory in directories: - self._volumes[directory] = {'bind': directory, 'mode': 'rw'} + absolute_dir = str(pathlib.Path(directory).absolute()) + self._volumes[absolute_dir] = {'bind': absolute_dir, 'mode': 'rw'} def add_environment_variables(self, variables): if self._container: diff --git a/tests/test_code_report.py b/tests/test_code_report.py index 9e57306c..f73cd2db 100644 --- a/tests/test_code_report.py +++ b/tests/test_code_report.py @@ -16,7 +16,7 @@ def fixture_runner_with_analyzers(docker_main: UniversumRunner): docker_main.environment.install_python_module("pylint") docker_main.environment.install_python_module("mypy") - docker_main.environment.assert_successful_execution("apt install uncrustify") + docker_main.environment.assert_successful_execution("apt install -y uncrustify clang-format") yield docker_main @@ -128,6 +128,11 @@ def finalize(self) -> str: input_tab_size = 2 """ +config_clang_format = """ +--- +AllowShortFunctionsOnASingleLine: Empty +""" + log_fail = r'Found [0-9]+ issues' log_success = r'Issues not found' @@ -167,7 +172,10 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c @pytest.mark.parametrize('analyzers, extra_args, tested_content, expected_success', [ [['uncrustify'], [], source_code_c, True], - [['uncrustify'], [], source_code_c.replace('\t', ' '), False], + [['uncrustify'], [], source_code_c.replace('\t', ' '), False], # by default uncrustify converts spaces to tabs + [['clang_format'], [], source_code_c.replace('\t', ' '), True], # by default clang-format expands tabs to 2 spaces + [['clang_format'], [], source_code_c.replace('\t', ' '), False], + [['clang_format', 'uncrustify'], [], source_code_c.replace('\t', ' '), False], [['pylint', 'mypy'], ["--python-version", python_version()], source_code_python, True], [['pylint'], ["--python-version", python_version()], source_code_python + '\n', False], [['mypy'], ["--python-version", python_version()], source_code_python.replace(': str', ': int'), False], @@ -177,6 +185,9 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c ], ids=[ 'uncrustify_no_issues', 'uncrustify_found_issues', + 'clang_format_no_issues', + 'clang_format_found_issues', + 'clang_format_and_uncrustify_found_issues', 'pylint_and_mypy_both_no_issues', 'pylint_found_issues', 'mypy_found_issues', @@ -194,6 +205,9 @@ def test_code_report_log(runner_with_analyzers: UniversumRunner, analyzers, extr if analyzer == 'uncrustify': args += ["--cfg-file", "cfg"] (runner_with_analyzers.local.root_directory / "cfg").write_text(config_uncrustify) + elif analyzer == 'clang_format': + (runner_with_analyzers.local.root_directory / ".clang-format").write_text(config_clang_format) + config.add_analyzer(analyzer, args) log = runner_with_analyzers.run(config.finalize()) @@ -210,7 +224,7 @@ def test_without_code_report_command(runner_with_analyzers: UniversumRunner): assert not pattern.findall(log) -@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify']) +@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify', 'clang_format']) @pytest.mark.parametrize('arg_set, expected_log', [ [["--files", "source_file.py"], "error: the following arguments are required: --result-file"], [["--files", "source_file.py", "--result-file"], "result-file: expected one argument"], @@ -235,10 +249,12 @@ def test_analyzer_python_version_params(runner_with_analyzers: UniversumRunner, "--result-file", "${CODE_REPORT_FILE}", '--rcfile'], "rcfile: expected one argument"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}"], - "Please specify the '--cfg_file' parameter or set an env. variable 'UNCRUSTIFY_CONFIG'"], + "Please specify the '--cfg-file' parameter or set 'UNCRUSTIFY_CONFIG' environment variable"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--cfg-file", "cfg", "--output-directory", "."], - "Target and source folders for uncrustify are not allowed to match"], + "Target folder must not be identical to source folder"], + ['clang_format', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "."], + "Target folder must not be identical to source folder"], ]) def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyzer, arg_set, expected_log): source_file = runner_with_analyzers.local.root_directory / "source_file" @@ -249,39 +265,54 @@ def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyz assert expected_log in log, f"'{expected_log}' is not found in '{log}'" -@pytest.mark.parametrize('extra_args, tested_content, expected_success, expected_artifact', [ - [[], source_code_c, True, False], - [["--report-html"], source_code_c.replace('\t', ' '), False, True], - [[], source_code_c.replace('\t', ' '), False, False], +@pytest.mark.parametrize('analyzer, extra_args, tested_content, expected_success, expected_artifact', [ + ['uncrustify', ["--report-html"], source_code_c, True, False], + ['uncrustify', ["--report-html"], source_code_c.replace('\t', ' '), False, True], + ['uncrustify', [], source_code_c.replace('\t', ' '), False, False], + ['clang_format', ["--report-html"], source_code_c.replace('\t', ' '), True, False], + ['clang_format', ["--report-html"], source_code_c, False, True], + ['clang_format', [], source_code_c, False, False], ], ids=[ "uncrustify_html_file_not_needed", "uncrustify_html_file_saved", "uncrustify_html_file_disabled", + "clang_format_html_file_not_needed", + "clang_format_html_file_saved", + "clang_format_html_file_disabled", ]) -def test_uncrustify_file_diff(runner_with_analyzers: UniversumRunner, - extra_args, tested_content, expected_success, expected_artifact): +def test_diff_html_file(runner_with_analyzers: UniversumRunner, analyzer, + extra_args, tested_content, expected_success, expected_artifact): + root = runner_with_analyzers.local.root_directory source_file = root / "source_file" source_file.write_text(tested_content) - (root / "cfg").write_text(config_uncrustify) common_args = [ "--result-file", "${CODE_REPORT_FILE}", "--files", "source_file", - "--cfg-file", "cfg", + "--output-directory", "diff_temp" ] + if analyzer == 'uncrustify': + (root / "cfg").write_text(config_uncrustify) + common_args.extend(["--cfg-file", "cfg"]) + elif analyzer == 'clang_format': + (root / ".clang-format").write_text(config_clang_format) + args = common_args + extra_args - extra_config = "artifacts='./uncrustify/source_file.html'" - log = runner_with_analyzers.run(ConfigData().add_analyzer('uncrustify', args, extra_config).finalize()) + extra_config = "artifacts='./diff_temp/source_file.html'" + log = runner_with_analyzers.run(ConfigData().add_analyzer(analyzer, args, extra_config).finalize()) 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_artifacts_state = "Success" if expected_artifact else "Failed" - expected_log = f"Collecting artifacts for the 'Run uncrustify' step - [^\n]*{expected_artifacts_state}" + expected_log = f"Collecting artifacts for the 'Run {analyzer}' step - [^\n]*{expected_artifacts_state}" assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" def test_code_report_extended_arg_search(tmp_path: pathlib.Path, stdout_checker: FuzzyCallChecker): + """ + Test if ${CODE_REPORT_FILE} is replaced not only in --result-file argument of the Step + """ env = utils.LocalTestEnvironment(tmp_path, "main") env.settings.Vcs.type = "none" env.settings.LocalMainVcs.source_dir = str(tmp_path) diff --git a/tests/test_nonci.py b/tests/test_nonci.py index fe24168d..d03bb4f0 100644 --- a/tests/test_nonci.py +++ b/tests/test_nonci.py @@ -23,7 +23,7 @@ def test_launcher_output(docker_nonci: UniversumRunner): - version control and review system are not used - project root is set to current directory """ - cwd = str(docker_nonci.local.root_directory) + cwd = str(docker_nonci.local.root_directory.absolute()) artifacts = docker_nonci.artifact_dir file_output_expected = f"Adding file {artifacts}/test_step_log.txt to artifacts" pwd_string_in_logs = f"pwd:[{cwd}]" diff --git a/universum/__init__.py b/universum/__init__.py index 97c18b90..efd4d5e5 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.15" +__version__ = "0.19.16" diff --git a/universum/analyzers/clang_format.py b/universum/analyzers/clang_format.py new file mode 100755 index 00000000..56a64f80 --- /dev/null +++ b/universum/analyzers/clang_format.py @@ -0,0 +1,67 @@ +import argparse +import pathlib +from typing import Callable, List, Optional, Tuple + +import yaml + +from . import utils, diff_utils + + +def clang_format_argument_parser() -> argparse.ArgumentParser: + parser = diff_utils.diff_analyzer_argument_parser("Clang-format analyzer", __file__, "clang-format") + parser.add_argument("--executable", "-e", dest="executable", default="clang-format", + help="The name of the clang-format executable, default: clang-format") + parser.add_argument("--style", dest="style", + help="The 'style' parameter of the clang-format. Can be literal 'file' string or " + "path to real file. See the clang-format documentation for details.") + return parser + + +def _add_style_param_if_present(cmd: List[str], settings: argparse.Namespace) -> None: + if settings.style: + cmd.extend(["-style", settings.style]) + + +@utils.sys_exit +@utils.analyzer(clang_format_argument_parser()) +def main(settings: argparse.Namespace) -> List[utils.ReportData]: + settings.name = "clang-format" + diff_utils.diff_analyzer_common_main(settings) + + html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None + if settings.write_html: + wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings) + html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) + + files: List[Tuple[pathlib.Path, pathlib.Path]] = [] + for src_file_absolute, target_file_absolute, _ in utils.get_files_with_absolute_paths(settings): + files.append((src_file_absolute, target_file_absolute)) + cmd = [settings.executable, src_file_absolute] + _add_style_param_if_present(cmd, settings) + output, _ = utils.run_for_output(cmd) + with open(target_file_absolute, "w", encoding="utf-8") as output_file: + output_file.write(output) + + return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) + + +def _get_wrapcolumn_tabsize(settings: argparse.Namespace) -> Tuple[int, int]: + cmd = [settings.executable, "--dump-config"] + _add_style_param_if_present(cmd, settings) + output, error = utils.run_for_output(cmd) + if error: + raise utils.AnalyzerException(message="clang-format --dump-config failed with the following error output: " + + error) + try: + clang_style = yaml.safe_load(output) + return clang_style["ColumnLimit"], clang_style["IndentWidth"] + except yaml.YAMLError as parse_error: + raise utils.AnalyzerException(message="Parsing of clang-format config produced the following error: " + + str(parse_error)) + except KeyError as key_error: + raise utils.AnalyzerException(message="Cannot find key in yaml during parsing of clang-format config: " + + str(key_error)) + + +if __name__ == "__main__": + main() diff --git a/universum/analyzers/diff_utils.py b/universum/analyzers/diff_utils.py new file mode 100644 index 00000000..493cf7de --- /dev/null +++ b/universum/analyzers/diff_utils.py @@ -0,0 +1,133 @@ +import argparse +import difflib +import pathlib +import shutil +from typing import Callable, List, Optional, Tuple + +from . import utils + + +class HtmlDiffFileWriter: + + def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: + self.target_folder = target_folder + self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) + + def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: + file_relative = file.relative_to(pathlib.Path.cwd()) + out_file_name: str = str(file_relative).replace('/', '_') + '.html' + with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: + out_file.write(self.differ.make_file(src, target, context=False)) + + +DiffWriter = Callable[[pathlib.Path, List[str], List[str]], None] + + +def diff_analyzer_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], + write_diff_file: Optional[DiffWriter] + ) -> List[utils.ReportData]: + result: List[utils.ReportData] = [] + for src_file, dst_file in files: + with open(src_file, encoding="utf-8") as src: + src_lines = src.readlines() + with open(dst_file, encoding="utf-8") as fixed: + fixed_lines = fixed.readlines() + + issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) + if issues and write_diff_file: + write_diff_file(src_file, src_lines, fixed_lines) + result.extend(issues) + return result + + +def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: + result = [] + matching_blocks: List[difflib.Match] = \ + difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() + previous_match = matching_blocks[0] + for match in matching_blocks[1:]: + block = _get_mismatching_block(previous_match, match, src, target) + previous_match = match + if not block: + continue + line, before, after = block + path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) + message = _get_issue_message(before, after) + result.append(utils.ReportData( + symbol="Code Style issue", + message=message, + path=str(path), + line=line + )) + + return result + + +def _get_issue_message(before: str, after: str) -> str: + # The maximum number of lines to write separate comments for + # If exceeded, summarized comment will be provided instead + max_lines = 11 + diff_size = len(before.splitlines()) + if diff_size > max_lines: + message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ + f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" + else: + # Message with before&after + message = f"\nOriginal code:\n```diff\n{before}```\n" + \ + f"Fixed code:\n```diff\n{after}```\n" + return message + + +def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] + match: difflib.Match, + src: List[str], target: List[str] + ) -> Optional[Tuple[int, str, str]]: + previous_match_end_in_src = previous_match.a + previous_match.size + previous_match_end_in_target = previous_match.b + previous_match.size + match_start_in_src = match.a + match_start_in_target = match.b + if previous_match_end_in_src == match_start_in_src: + return None + line = match_start_in_src + before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) + after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) + return line, before, after + + +def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: + return _replace_whitespace_characters(''.join(lines[start: end])) + + +_whitespace_character_mapping = { + " ": "\u00b7", + "\t": "\u2192\u2192\u2192\u2192", + "\n": "\u2193\u000a" +}.items() + + +def _replace_whitespace_characters(line: str) -> str: + for old_str, new_str in _whitespace_character_mapping: + line = line.replace(old_str, new_str) + return line + + +def diff_analyzer_argument_parser(description: str, module_path: str, output_directory: str) -> argparse.ArgumentParser: + parser = utils.create_parser(description, module_path) + parser.add_argument("--output-directory", "-od", dest="output_directory", default=output_directory, + help=f"Directory to store fixed files and HTML files with diff; the default " + f"value is '{output_directory}'. Has to be distinct from source directory") + parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, + help="(optional) Set to generate html reports for each modified file") + return parser + + +def diff_analyzer_common_main(settings: argparse.Namespace) -> None: + settings.target_folder = utils.normalize_path(settings.output_directory) + if settings.target_folder.exists() and settings.target_folder.samefile(pathlib.Path.cwd()): + raise EnvironmentError("Target folder must not be identical to source folder") + + settings.target_folder.mkdir(parents=True, exist_ok=True) + + if not shutil.which(settings.executable): + raise EnvironmentError(f"{settings.name} executable '{settings.executable}' is not found. " + f"Please install {settings.name} or fix the executable name.") diff --git a/universum/analyzers/mypy.py b/universum/analyzers/mypy.py index d5c95e54..a8b0544a 100644 --- a/universum/analyzers/mypy.py +++ b/universum/analyzers/mypy.py @@ -1,12 +1,11 @@ import argparse - from typing import List from . import utils def mypy_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Mypy analyzer") + parser = utils.create_parser("Mypy analyzer", __file__) parser.add_argument("--config-file", dest="config_file", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/pylint.py b/universum/analyzers/pylint.py index 7b383f89..11cb8a67 100644 --- a/universum/analyzers/pylint.py +++ b/universum/analyzers/pylint.py @@ -1,13 +1,12 @@ import argparse import json - from typing import List from . import utils def pylint_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Pylint analyzer") + parser = utils.create_parser("Pylint analyzer", __file__) parser.add_argument("--rcfile", dest="rcfile", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index 2f24ca2d..7afc548d 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -1,86 +1,44 @@ import argparse -import difflib import os -import shutil import pathlib import re - from typing import Callable, List, Optional, Tuple -from . import utils +from . import utils, diff_utils def uncrustify_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Uncrustify analyzer") + parser = diff_utils.diff_analyzer_argument_parser("Uncrustify analyzer", __file__, "uncrustify") parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") - parser.add_argument("--output-directory", "-od", dest="output_directory", default="uncrustify", - help="Directory to store fixed files, generated by Uncrustify " - "and HTML files with diff; the default value is 'uncrustify'" - "Has to be distinct from source directory") - parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, - help="(optional) Set to generate html reports for each modified file") return parser @utils.sys_exit @utils.analyzer(uncrustify_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: - if not shutil.which('uncrustify'): - raise EnvironmentError("Please install uncrustify") + settings.name = "uncrustify" + settings.executable = "uncrustify" + diff_utils.diff_analyzer_common_main(settings) + if not settings.cfg_file and 'UNCRUSTIFY_CONFIG' not in os.environ: - raise EnvironmentError("Please specify the '--cfg_file' parameter " - "or set an env. variable 'UNCRUSTIFY_CONFIG'") - target_folder: pathlib.Path = utils.normalize(settings.output_directory) - if target_folder.exists() and target_folder.samefile(pathlib.Path.cwd()): - raise EnvironmentError("Target and source folders for uncrustify are not allowed to match") + raise EnvironmentError("Please specify the '--cfg-file' parameter " + "or set 'UNCRUSTIFY_CONFIG' environment variable") + html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None if settings.write_html: wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings.cfg_file) - html_diff_file_writer = HtmlDiffFileWriter(target_folder, wrapcolumn, tabsize) + html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) cmd = ["uncrustify", "-q", "-c", settings.cfg_file, "--prefix", settings.output_directory] files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file in settings.file_list: - src_file_absolute = utils.normalize(src_file) - src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) - target_file_absolute: pathlib.Path = target_folder.joinpath(src_file_relative) + for src_file_absolute, target_file_absolute, src_file_relative in utils.get_files_with_absolute_paths(settings): files.append((src_file_absolute, target_file_absolute)) cmd.append(src_file_relative) utils.run_for_output(cmd) - return uncrustify_output_parser(files, html_diff_file_writer) - - -class HtmlDiffFileWriter: - - def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: - self.target_folder = target_folder - self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) - - def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: - file_relative = file.relative_to(pathlib.Path.cwd()) - out_file_name: str = str(file_relative).replace('/', '_') + '.html' - with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: - out_file.write(self.differ.make_file(src, target, context=False)) - - -def uncrustify_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], - write_diff_file: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] - ) -> List[utils.ReportData]: - result: List[utils.ReportData] = [] - for src_file, uncrustify_file in files: - with open(src_file, encoding="utf-8") as src: - src_lines = src.readlines() - with open(uncrustify_file, encoding="utf-8") as fixed: - fixed_lines = fixed.readlines() - - issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) - if issues and write_diff_file: - write_diff_file(src_file, src_lines, fixed_lines) - result.extend(issues) - return result + return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: @@ -99,69 +57,5 @@ def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: return wrapcolumn, tabsize -def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: - result = [] - matching_blocks: List[difflib.Match] = \ - difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() - previous_match = matching_blocks[0] - for match in matching_blocks[1:]: - block = _get_mismatching_block(previous_match, match, src, target) - previous_match = match - if not block: - continue - line, before, after = block - path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) - message = _get_issue_message(before, after) - result.append(utils.ReportData( - symbol="Uncrustify Code Style issue", - message=message, - path=str(path), - line=line - )) - - return result - - -def _get_issue_message(before: str, after: str) -> str: - # The maximum number of lines to write separate comments for - # If exceeded, summarized comment will be provided instead - max_lines = 11 - diff_size = len(before.splitlines()) - if diff_size > max_lines: - message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ - f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" - else: - # Message with before&after - message = f"\nOriginal code:\n```diff\n{before}```\n" + \ - f"Uncrustify generated code:\n```diff\n{after}```\n" - return message - - -def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] - match: difflib.Match, - src: List[str], target: List[str] - ) -> Optional[Tuple[int, str, str]]: - previous_match_end_in_src = previous_match.a + previous_match.size - previous_match_end_in_target = previous_match.b + previous_match.size - match_start_in_src = match.a - match_start_in_target = match.b - if previous_match_end_in_src == match_start_in_src: - return None - line = match_start_in_src - before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) - after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) - return line, before, after - - -def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: - return _replace_invisible_symbols(''.join(lines[start: end])) - - -def _replace_invisible_symbols(line: str) -> str: - for old_str, new_str in zip([" ", "\t", "\n"], ["\u00b7", "\u2192\u2192\u2192\u2192", "\u2193\u000a"]): - line = line.replace(old_str, new_str) - return line - - if __name__ == "__main__": - main() # pylint: disable=no-value-for-parameter # see https://github.com/PyCQA/pylint/issues/259 + main() diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index a6bfd452..7d921709 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -1,11 +1,12 @@ -import json -import sys import argparse import glob +import json +import os import pathlib import subprocess +import sys +from typing import Any, Callable, List, Optional, Tuple, Set, Iterable -from typing import Any, Callable, List, Optional, Tuple, Set from typing_extensions import TypedDict from universum.lib.ci_exception import CiException @@ -19,6 +20,13 @@ def __init__(self, code: int = 2, message: Optional[str] = None): self.message: Optional[str] = message +def create_parser(description: str, module_path: str) -> argparse.ArgumentParser: + module_name, _ = os.path.splitext(os.path.basename(module_path)) + + prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {__package__}.{module_name}" + return argparse.ArgumentParser(prog=prog, description=description) + + def analyzer(parser: argparse.ArgumentParser): """ Wraps the analyzer specific data and adds common protocol information: @@ -29,6 +37,7 @@ def analyzer(parser: argparse.ArgumentParser): :param parser: Definition of analyzer custom arguments :return: Wrapped analyzer with common reporting behaviour """ + def internal(func: Callable[[argparse.Namespace], List[ReportData]]) -> Callable[[], List[ReportData]]: def wrapper() -> List[ReportData]: add_files_argument(parser) @@ -75,6 +84,7 @@ def sys_exit(func: Callable[[], Any]) -> Callable[[], None]: >>> wrap_system_exit(sys_exit(_raise_custom)) 3 """ + def wrapper() -> None: exit_code: int try: @@ -146,6 +156,16 @@ def report_to_file(issues: List[ReportData], json_file: Optional[str] = None) -> sys.stdout.write(issues_json) -def normalize(file: str) -> pathlib.Path: +def normalize_path(file: str) -> pathlib.Path: file_path = pathlib.Path(file) return file_path if file_path.is_absolute() else pathlib.Path.cwd().joinpath(file_path) + + +def get_files_with_absolute_paths(settings: argparse.Namespace) -> Iterable[Tuple[pathlib.Path, + pathlib.Path, + pathlib.Path]]: + for src_file in settings.file_list: + src_file_absolute = normalize_path(src_file) + src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) + target_file_absolute: pathlib.Path = settings.target_folder.joinpath(src_file_relative) + yield src_file_absolute, target_file_absolute, src_file_relative diff --git a/universum/configuration_support.py b/universum/configuration_support.py index bf798af8..efd76872 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -355,7 +355,7 @@ def get(self, key: str, default: Any = None) -> Any: ... warnings.simplefilter("always") ... f() ... return w - >>> step = Step(name='foo', my_var='bar') + >>> step = Step(name='foo', my_var='bar', t1=None, t2=False) >>> do_and_get_warnings(lambda : step.get('name', 'test')) # doctest: +ELLIPSIS [] @@ -367,12 +367,16 @@ def get(self, key: str, default: Any = None) -> Any: 'test' >>> step.get('command', 'test') 'test' + >>> step.get('t1') is None + True + >>> step.get('t2') + False """ result = self._extras.get(key) - if result: + if result is not None: # for custom fields there is a distinction between None and falsy values return result result = self.__dict__.get(key) - if result: + if result: # non-custom fields initialized with falsy values warn("Using legacy API to access configuration values. Please use var." + key + " instead.") return result return default diff --git a/universum/modules/code_report_collector.py b/universum/modules/code_report_collector.py index e558814c..edb2e795 100644 --- a/universum/modules/code_report_collector.py +++ b/universum/modules/code_report_collector.py @@ -92,9 +92,6 @@ def report_code_report_results(self) -> None: if text: report = json.loads(text) - json_file: TextIO = self.artifacts.create_text_file("Static_analysis_report.json") - json_file.write(json.dumps(report, indent=4)) - issue_count: int if not report and report != []: self.out.log_error("There are no results in code report file. Something went wrong.") diff --git a/universum/modules/vcs/swarm.py b/universum/modules/vcs/swarm.py index 53eb8e46..ca671301 100644 --- a/universum/modules/vcs/swarm.py +++ b/universum/modules/vcs/swarm.py @@ -9,7 +9,7 @@ from ...lib.ci_exception import CiException from ...lib.gravity import Dependency -urllib3.disable_warnings((urllib3.exceptions.InsecurePlatformWarning, urllib3.exceptions.SNIMissingWarning)) # type: ignore +urllib3.disable_warnings(urllib3.exceptions.InsecurePlatformWarning) # type: ignore __all__ = [ "Swarm" From c657ee91f1c9061be1d45bf5a4aab61a7dc8dbfe Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 25 May 2023 21:47:04 +0300 Subject: [PATCH 15/25] Revert "chore: merge changes from master (#803)" (#807) This reverts commit fcfdf6be550bfc5866afec59ecd0743f372e239a. --- .github/workflows/telegram-bot.yml | 8 +- CHANGELOG.rst | 17 --- doc/code_report.rst | 42 +------ doc/conf.py | 2 +- pylintrc | 5 +- setup.py | 7 +- tests/deployment_utils.py | 3 +- tests/test_code_report.py | 63 +++------- tests/test_nonci.py | 2 +- universum/__init__.py | 2 +- universum/analyzers/clang_format.py | 67 ----------- universum/analyzers/diff_utils.py | 133 --------------------- universum/analyzers/mypy.py | 3 +- universum/analyzers/pylint.py | 3 +- universum/analyzers/uncrustify.py | 132 ++++++++++++++++++-- universum/analyzers/utils.py | 28 +---- universum/configuration_support.py | 10 +- universum/modules/code_report_collector.py | 3 + universum/modules/vcs/swarm.py | 2 +- 19 files changed, 164 insertions(+), 368 deletions(-) delete mode 100755 universum/analyzers/clang_format.py delete mode 100644 universum/analyzers/diff_utils.py diff --git a/.github/workflows/telegram-bot.yml b/.github/workflows/telegram-bot.yml index c319b739..6d623abc 100644 --- a/.github/workflows/telegram-bot.yml +++ b/.github/workflows/telegram-bot.yml @@ -23,7 +23,6 @@ jobs: PR_MERGED: ${{ github.event.pull_request.merged_by.login }} REVIEW_STATE: ${{ github.event.review.state }} REVIEW_AUTHOR: ${{ github.event.review.user.login }} - REVIEW_COMMENT: ${{ github.event.review.body }} COMMENT_AUTHOR: ${{ github.event.comment.user.login }} COMMENT_URL: ${{ github.event.comment.html_url }} COMMENT_BODY: ${{ github.event.comment.body }} @@ -52,10 +51,7 @@ jobs: elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} requested changes for PR#${{ env.PR_NUMBER }}"` - elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "commented" ]]; then - ESCAPED_TEXT=`echo -e "${{ env.REVIEW_COMMENT }}"| sed 's/\&/\&/g' | sed 's//\>/g'` - TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} posted the following comment to PR#${{ env.PR_NUMBER }}:\n$ESCAPED_TEXT"` - elif [[ ! -z "${{ github.event.review }}" ]]; then + elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" != "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} ${{ env.REVIEW_STATE }} PR#${{ env.PR_NUMBER }}"` elif [[ -z "${{ github.event.review }}" && "${{ github.event.action }}" == "submitted" ]]; then TEXT=`echo -e "Due to GitHub Actions bug we cannot identify, who approved PR#${{ env.PR_NUMBER }}"` @@ -67,7 +63,7 @@ jobs: fi if [[ ! -z $TEXT ]]; then - curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" --data-urlencode "disable_web_page_preview=True" \ + curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" \ --data-urlencode "text=$TEXT" --data-urlencode "parse_mode=HTML" $URL fi diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0678da9b..0ea21a2d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,23 +1,6 @@ Change log ========== -0.19.15 (2023-05-10) --------------------- - -New features -~~~~~~~~~~~~ - -* **analyzer:** code report based on clang-format - -Bug fixes -~~~~~~~~~ - -* **config:** fixed returning "None" instead of "False" for Step custom keys set to "False" -* **artifact:** remove redundant Static_analysis_report.json from artifacts -* **analyzer:** incorrect program name in help -* **report:** set exit code even if reporting crashes - - 0.19.14 (2022-11-14) -------------------- diff --git a/doc/code_report.rst b/doc/code_report.rst index 48317dcf..a9ca4b86 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -6,7 +6,6 @@ The following analysing modules (analysers) are currently added to Universum: * `pylint`_ * `mypy`_ * `uncrustify`_ - * `clang-format`_ Analysers are separate scripts, fully compatible with Universum. It is possible to use them as independent Python modules. @@ -63,6 +62,7 @@ Pylint .. argparse:: :ref: universum.analyzers.pylint.pylint_argument_parser + :prog: {python} -m universum.analyzers.pylint Config example for ``universum.analyzers.pylint``: @@ -99,6 +99,7 @@ Mypy .. argparse:: :ref: universum.analyzers.mypy.mypy_argument_parser + :prog: {python} -m universum.analyzers.mypy Config example for ``universum.analyzers.mypy``: @@ -135,6 +136,7 @@ Uncrustify .. argparse:: :ref: universum.analyzers.uncrustify.uncrustify_argument_parser + :prog: {python} -m universum.analyzers.uncrustify :nodefault: Config example for ``universum.analyzers.uncrustify``: @@ -144,7 +146,7 @@ Config example for ``universum.analyzers.uncrustify``: from universum.configuration_support import Configuration, Step configs = Configuration([Step(name="uncrustify", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp/*.c", + "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp", "--cfg-file", "file_name.cfg", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "uncrustify" ])]) @@ -162,38 +164,4 @@ will produce this list of configurations: .. testoutput:: $ ./.universum.py - [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp/*.c --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] - -Clang-format ------------- - -.. argparse:: - :ref: universum.analyzers.clang_format.clang_format_argument_parser - :nodefault: - -Config example for ``universum.analyzers.clang_format``: - -.. testcode:: - - from universum.configuration_support import Configuration, Step - - configs = Configuration([Step(name="clang-format", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.clang-format", "--files", "/home/user/workspace/temp/*.c", - "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "clang-format", "-e", "clang-format-15", - ])]) - - if __name__ == '__main__': - print(configs.dump()) - -will produce this list of configurations: - -.. testcode:: - :hide: - - print("$ ./.universum.py") - print(configs.dump()) - -.. testoutput:: - - $ ./.universum.py - [{'name': 'clang-format', 'code_report': True, 'command': '{python} -m universum.analyzers.clang-format --files /home/user/workspace/temp/*.c --result-file ${CODE_REPORT_FILE} --output-directory clang-format -e clang-format-15'}] + [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] diff --git a/doc/conf.py b/doc/conf.py index d25e3408..229cdb94 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -50,7 +50,7 @@ # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = 'en' +language = None # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. diff --git a/pylintrc b/pylintrc index ee326fca..7b39d854 100644 --- a/pylintrc +++ b/pylintrc @@ -37,7 +37,4 @@ max-parents = 10 max-line-length = 130 [SIMILARITIES] -ignore-imports=yes - -[TYPECHECK] -signature-mutators=universum.analyzers.utils.analyzer +ignore-imports=yes \ No newline at end of file diff --git a/setup.py b/setup.py index e5dc59d4..642aa26c 100644 --- a/setup.py +++ b/setup.py @@ -35,8 +35,7 @@ def readme(): 'sh', 'lxml', 'typing-extensions', - 'ansi2html', - 'pyyaml==6.0' + 'ansi2html' ], extras_require={ 'p4': [p4], @@ -57,9 +56,7 @@ def readme(): 'coverage', 'mypy', 'types-requests', - 'selenium==3.141', - 'urllib3==1.26.15', # This is required for selenium-3.141 to work correctly - 'types-PyYAML==6.0' + 'selenium==3.141' ] }, package_data={'': ['*.css', '*.js']} diff --git a/tests/deployment_utils.py b/tests/deployment_utils.py index 32a67194..8d080315 100644 --- a/tests/deployment_utils.py +++ b/tests/deployment_utils.py @@ -42,8 +42,7 @@ def add_bind_dirs(self, directories): if self._container: self.request.raiseerror("Container is already running, no dirs can be bound!") for directory in directories: - absolute_dir = str(pathlib.Path(directory).absolute()) - self._volumes[absolute_dir] = {'bind': absolute_dir, 'mode': 'rw'} + self._volumes[directory] = {'bind': directory, 'mode': 'rw'} def add_environment_variables(self, variables): if self._container: diff --git a/tests/test_code_report.py b/tests/test_code_report.py index f73cd2db..9e57306c 100644 --- a/tests/test_code_report.py +++ b/tests/test_code_report.py @@ -16,7 +16,7 @@ def fixture_runner_with_analyzers(docker_main: UniversumRunner): docker_main.environment.install_python_module("pylint") docker_main.environment.install_python_module("mypy") - docker_main.environment.assert_successful_execution("apt install -y uncrustify clang-format") + docker_main.environment.assert_successful_execution("apt install uncrustify") yield docker_main @@ -128,11 +128,6 @@ def finalize(self) -> str: input_tab_size = 2 """ -config_clang_format = """ ---- -AllowShortFunctionsOnASingleLine: Empty -""" - log_fail = r'Found [0-9]+ issues' log_success = r'Issues not found' @@ -172,10 +167,7 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c @pytest.mark.parametrize('analyzers, extra_args, tested_content, expected_success', [ [['uncrustify'], [], source_code_c, True], - [['uncrustify'], [], source_code_c.replace('\t', ' '), False], # by default uncrustify converts spaces to tabs - [['clang_format'], [], source_code_c.replace('\t', ' '), True], # by default clang-format expands tabs to 2 spaces - [['clang_format'], [], source_code_c.replace('\t', ' '), False], - [['clang_format', 'uncrustify'], [], source_code_c.replace('\t', ' '), False], + [['uncrustify'], [], source_code_c.replace('\t', ' '), False], [['pylint', 'mypy'], ["--python-version", python_version()], source_code_python, True], [['pylint'], ["--python-version", python_version()], source_code_python + '\n', False], [['mypy'], ["--python-version", python_version()], source_code_python.replace(': str', ': int'), False], @@ -185,9 +177,6 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c ], ids=[ 'uncrustify_no_issues', 'uncrustify_found_issues', - 'clang_format_no_issues', - 'clang_format_found_issues', - 'clang_format_and_uncrustify_found_issues', 'pylint_and_mypy_both_no_issues', 'pylint_found_issues', 'mypy_found_issues', @@ -205,9 +194,6 @@ def test_code_report_log(runner_with_analyzers: UniversumRunner, analyzers, extr if analyzer == 'uncrustify': args += ["--cfg-file", "cfg"] (runner_with_analyzers.local.root_directory / "cfg").write_text(config_uncrustify) - elif analyzer == 'clang_format': - (runner_with_analyzers.local.root_directory / ".clang-format").write_text(config_clang_format) - config.add_analyzer(analyzer, args) log = runner_with_analyzers.run(config.finalize()) @@ -224,7 +210,7 @@ def test_without_code_report_command(runner_with_analyzers: UniversumRunner): assert not pattern.findall(log) -@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify', 'clang_format']) +@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify']) @pytest.mark.parametrize('arg_set, expected_log', [ [["--files", "source_file.py"], "error: the following arguments are required: --result-file"], [["--files", "source_file.py", "--result-file"], "result-file: expected one argument"], @@ -249,12 +235,10 @@ def test_analyzer_python_version_params(runner_with_analyzers: UniversumRunner, "--result-file", "${CODE_REPORT_FILE}", '--rcfile'], "rcfile: expected one argument"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}"], - "Please specify the '--cfg-file' parameter or set 'UNCRUSTIFY_CONFIG' environment variable"], + "Please specify the '--cfg_file' parameter or set an env. variable 'UNCRUSTIFY_CONFIG'"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--cfg-file", "cfg", "--output-directory", "."], - "Target folder must not be identical to source folder"], - ['clang_format', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "."], - "Target folder must not be identical to source folder"], + "Target and source folders for uncrustify are not allowed to match"], ]) def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyzer, arg_set, expected_log): source_file = runner_with_analyzers.local.root_directory / "source_file" @@ -265,54 +249,39 @@ def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyz assert expected_log in log, f"'{expected_log}' is not found in '{log}'" -@pytest.mark.parametrize('analyzer, extra_args, tested_content, expected_success, expected_artifact', [ - ['uncrustify', ["--report-html"], source_code_c, True, False], - ['uncrustify', ["--report-html"], source_code_c.replace('\t', ' '), False, True], - ['uncrustify', [], source_code_c.replace('\t', ' '), False, False], - ['clang_format', ["--report-html"], source_code_c.replace('\t', ' '), True, False], - ['clang_format', ["--report-html"], source_code_c, False, True], - ['clang_format', [], source_code_c, False, False], +@pytest.mark.parametrize('extra_args, tested_content, expected_success, expected_artifact', [ + [[], source_code_c, True, False], + [["--report-html"], source_code_c.replace('\t', ' '), False, True], + [[], source_code_c.replace('\t', ' '), False, False], ], ids=[ "uncrustify_html_file_not_needed", "uncrustify_html_file_saved", "uncrustify_html_file_disabled", - "clang_format_html_file_not_needed", - "clang_format_html_file_saved", - "clang_format_html_file_disabled", ]) -def test_diff_html_file(runner_with_analyzers: UniversumRunner, analyzer, - extra_args, tested_content, expected_success, expected_artifact): - +def test_uncrustify_file_diff(runner_with_analyzers: UniversumRunner, + extra_args, tested_content, expected_success, expected_artifact): root = runner_with_analyzers.local.root_directory source_file = root / "source_file" source_file.write_text(tested_content) + (root / "cfg").write_text(config_uncrustify) common_args = [ "--result-file", "${CODE_REPORT_FILE}", "--files", "source_file", - "--output-directory", "diff_temp" + "--cfg-file", "cfg", ] - if analyzer == 'uncrustify': - (root / "cfg").write_text(config_uncrustify) - common_args.extend(["--cfg-file", "cfg"]) - elif analyzer == 'clang_format': - (root / ".clang-format").write_text(config_clang_format) - args = common_args + extra_args - extra_config = "artifacts='./diff_temp/source_file.html'" - log = runner_with_analyzers.run(ConfigData().add_analyzer(analyzer, args, extra_config).finalize()) + extra_config = "artifacts='./uncrustify/source_file.html'" + log = runner_with_analyzers.run(ConfigData().add_analyzer('uncrustify', args, extra_config).finalize()) 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_artifacts_state = "Success" if expected_artifact else "Failed" - expected_log = f"Collecting artifacts for the 'Run {analyzer}' step - [^\n]*{expected_artifacts_state}" + 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}'" def test_code_report_extended_arg_search(tmp_path: pathlib.Path, stdout_checker: FuzzyCallChecker): - """ - Test if ${CODE_REPORT_FILE} is replaced not only in --result-file argument of the Step - """ env = utils.LocalTestEnvironment(tmp_path, "main") env.settings.Vcs.type = "none" env.settings.LocalMainVcs.source_dir = str(tmp_path) diff --git a/tests/test_nonci.py b/tests/test_nonci.py index d03bb4f0..fe24168d 100644 --- a/tests/test_nonci.py +++ b/tests/test_nonci.py @@ -23,7 +23,7 @@ def test_launcher_output(docker_nonci: UniversumRunner): - version control and review system are not used - project root is set to current directory """ - cwd = str(docker_nonci.local.root_directory.absolute()) + cwd = str(docker_nonci.local.root_directory) artifacts = docker_nonci.artifact_dir file_output_expected = f"Adding file {artifacts}/test_step_log.txt to artifacts" pwd_string_in_logs = f"pwd:[{cwd}]" diff --git a/universum/__init__.py b/universum/__init__.py index efd4d5e5..97c18b90 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.16" +__version__ = "0.19.15" diff --git a/universum/analyzers/clang_format.py b/universum/analyzers/clang_format.py deleted file mode 100755 index 56a64f80..00000000 --- a/universum/analyzers/clang_format.py +++ /dev/null @@ -1,67 +0,0 @@ -import argparse -import pathlib -from typing import Callable, List, Optional, Tuple - -import yaml - -from . import utils, diff_utils - - -def clang_format_argument_parser() -> argparse.ArgumentParser: - parser = diff_utils.diff_analyzer_argument_parser("Clang-format analyzer", __file__, "clang-format") - parser.add_argument("--executable", "-e", dest="executable", default="clang-format", - help="The name of the clang-format executable, default: clang-format") - parser.add_argument("--style", dest="style", - help="The 'style' parameter of the clang-format. Can be literal 'file' string or " - "path to real file. See the clang-format documentation for details.") - return parser - - -def _add_style_param_if_present(cmd: List[str], settings: argparse.Namespace) -> None: - if settings.style: - cmd.extend(["-style", settings.style]) - - -@utils.sys_exit -@utils.analyzer(clang_format_argument_parser()) -def main(settings: argparse.Namespace) -> List[utils.ReportData]: - settings.name = "clang-format" - diff_utils.diff_analyzer_common_main(settings) - - html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None - if settings.write_html: - wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings) - html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) - - files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file_absolute, target_file_absolute, _ in utils.get_files_with_absolute_paths(settings): - files.append((src_file_absolute, target_file_absolute)) - cmd = [settings.executable, src_file_absolute] - _add_style_param_if_present(cmd, settings) - output, _ = utils.run_for_output(cmd) - with open(target_file_absolute, "w", encoding="utf-8") as output_file: - output_file.write(output) - - return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) - - -def _get_wrapcolumn_tabsize(settings: argparse.Namespace) -> Tuple[int, int]: - cmd = [settings.executable, "--dump-config"] - _add_style_param_if_present(cmd, settings) - output, error = utils.run_for_output(cmd) - if error: - raise utils.AnalyzerException(message="clang-format --dump-config failed with the following error output: " + - error) - try: - clang_style = yaml.safe_load(output) - return clang_style["ColumnLimit"], clang_style["IndentWidth"] - except yaml.YAMLError as parse_error: - raise utils.AnalyzerException(message="Parsing of clang-format config produced the following error: " + - str(parse_error)) - except KeyError as key_error: - raise utils.AnalyzerException(message="Cannot find key in yaml during parsing of clang-format config: " + - str(key_error)) - - -if __name__ == "__main__": - main() diff --git a/universum/analyzers/diff_utils.py b/universum/analyzers/diff_utils.py deleted file mode 100644 index 493cf7de..00000000 --- a/universum/analyzers/diff_utils.py +++ /dev/null @@ -1,133 +0,0 @@ -import argparse -import difflib -import pathlib -import shutil -from typing import Callable, List, Optional, Tuple - -from . import utils - - -class HtmlDiffFileWriter: - - def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: - self.target_folder = target_folder - self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) - - def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: - file_relative = file.relative_to(pathlib.Path.cwd()) - out_file_name: str = str(file_relative).replace('/', '_') + '.html' - with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: - out_file.write(self.differ.make_file(src, target, context=False)) - - -DiffWriter = Callable[[pathlib.Path, List[str], List[str]], None] - - -def diff_analyzer_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], - write_diff_file: Optional[DiffWriter] - ) -> List[utils.ReportData]: - result: List[utils.ReportData] = [] - for src_file, dst_file in files: - with open(src_file, encoding="utf-8") as src: - src_lines = src.readlines() - with open(dst_file, encoding="utf-8") as fixed: - fixed_lines = fixed.readlines() - - issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) - if issues and write_diff_file: - write_diff_file(src_file, src_lines, fixed_lines) - result.extend(issues) - return result - - -def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: - result = [] - matching_blocks: List[difflib.Match] = \ - difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() - previous_match = matching_blocks[0] - for match in matching_blocks[1:]: - block = _get_mismatching_block(previous_match, match, src, target) - previous_match = match - if not block: - continue - line, before, after = block - path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) - message = _get_issue_message(before, after) - result.append(utils.ReportData( - symbol="Code Style issue", - message=message, - path=str(path), - line=line - )) - - return result - - -def _get_issue_message(before: str, after: str) -> str: - # The maximum number of lines to write separate comments for - # If exceeded, summarized comment will be provided instead - max_lines = 11 - diff_size = len(before.splitlines()) - if diff_size > max_lines: - message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ - f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" - else: - # Message with before&after - message = f"\nOriginal code:\n```diff\n{before}```\n" + \ - f"Fixed code:\n```diff\n{after}```\n" - return message - - -def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] - match: difflib.Match, - src: List[str], target: List[str] - ) -> Optional[Tuple[int, str, str]]: - previous_match_end_in_src = previous_match.a + previous_match.size - previous_match_end_in_target = previous_match.b + previous_match.size - match_start_in_src = match.a - match_start_in_target = match.b - if previous_match_end_in_src == match_start_in_src: - return None - line = match_start_in_src - before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) - after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) - return line, before, after - - -def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: - return _replace_whitespace_characters(''.join(lines[start: end])) - - -_whitespace_character_mapping = { - " ": "\u00b7", - "\t": "\u2192\u2192\u2192\u2192", - "\n": "\u2193\u000a" -}.items() - - -def _replace_whitespace_characters(line: str) -> str: - for old_str, new_str in _whitespace_character_mapping: - line = line.replace(old_str, new_str) - return line - - -def diff_analyzer_argument_parser(description: str, module_path: str, output_directory: str) -> argparse.ArgumentParser: - parser = utils.create_parser(description, module_path) - parser.add_argument("--output-directory", "-od", dest="output_directory", default=output_directory, - help=f"Directory to store fixed files and HTML files with diff; the default " - f"value is '{output_directory}'. Has to be distinct from source directory") - parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, - help="(optional) Set to generate html reports for each modified file") - return parser - - -def diff_analyzer_common_main(settings: argparse.Namespace) -> None: - settings.target_folder = utils.normalize_path(settings.output_directory) - if settings.target_folder.exists() and settings.target_folder.samefile(pathlib.Path.cwd()): - raise EnvironmentError("Target folder must not be identical to source folder") - - settings.target_folder.mkdir(parents=True, exist_ok=True) - - if not shutil.which(settings.executable): - raise EnvironmentError(f"{settings.name} executable '{settings.executable}' is not found. " - f"Please install {settings.name} or fix the executable name.") diff --git a/universum/analyzers/mypy.py b/universum/analyzers/mypy.py index a8b0544a..d5c95e54 100644 --- a/universum/analyzers/mypy.py +++ b/universum/analyzers/mypy.py @@ -1,11 +1,12 @@ import argparse + from typing import List from . import utils def mypy_argument_parser() -> argparse.ArgumentParser: - parser = utils.create_parser("Mypy analyzer", __file__) + parser = argparse.ArgumentParser(description="Mypy analyzer") parser.add_argument("--config-file", dest="config_file", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/pylint.py b/universum/analyzers/pylint.py index 11cb8a67..7b383f89 100644 --- a/universum/analyzers/pylint.py +++ b/universum/analyzers/pylint.py @@ -1,12 +1,13 @@ import argparse import json + from typing import List from . import utils def pylint_argument_parser() -> argparse.ArgumentParser: - parser = utils.create_parser("Pylint analyzer", __file__) + parser = argparse.ArgumentParser(description="Pylint analyzer") parser.add_argument("--rcfile", dest="rcfile", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index 7afc548d..2f24ca2d 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -1,44 +1,86 @@ import argparse +import difflib import os +import shutil import pathlib import re + from typing import Callable, List, Optional, Tuple -from . import utils, diff_utils +from . import utils def uncrustify_argument_parser() -> argparse.ArgumentParser: - parser = diff_utils.diff_analyzer_argument_parser("Uncrustify analyzer", __file__, "uncrustify") + parser = argparse.ArgumentParser(description="Uncrustify analyzer") parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") + parser.add_argument("--output-directory", "-od", dest="output_directory", default="uncrustify", + help="Directory to store fixed files, generated by Uncrustify " + "and HTML files with diff; the default value is 'uncrustify'" + "Has to be distinct from source directory") + parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, + help="(optional) Set to generate html reports for each modified file") return parser @utils.sys_exit @utils.analyzer(uncrustify_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: - settings.name = "uncrustify" - settings.executable = "uncrustify" - diff_utils.diff_analyzer_common_main(settings) - + if not shutil.which('uncrustify'): + raise EnvironmentError("Please install uncrustify") if not settings.cfg_file and 'UNCRUSTIFY_CONFIG' not in os.environ: - raise EnvironmentError("Please specify the '--cfg-file' parameter " - "or set 'UNCRUSTIFY_CONFIG' environment variable") - + raise EnvironmentError("Please specify the '--cfg_file' parameter " + "or set an env. variable 'UNCRUSTIFY_CONFIG'") + target_folder: pathlib.Path = utils.normalize(settings.output_directory) + if target_folder.exists() and target_folder.samefile(pathlib.Path.cwd()): + raise EnvironmentError("Target and source folders for uncrustify are not allowed to match") html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None if settings.write_html: wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings.cfg_file) - html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) + html_diff_file_writer = HtmlDiffFileWriter(target_folder, wrapcolumn, tabsize) cmd = ["uncrustify", "-q", "-c", settings.cfg_file, "--prefix", settings.output_directory] files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file_absolute, target_file_absolute, src_file_relative in utils.get_files_with_absolute_paths(settings): + for src_file in settings.file_list: + src_file_absolute = utils.normalize(src_file) + src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) + target_file_absolute: pathlib.Path = target_folder.joinpath(src_file_relative) files.append((src_file_absolute, target_file_absolute)) cmd.append(src_file_relative) utils.run_for_output(cmd) - return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) + return uncrustify_output_parser(files, html_diff_file_writer) + + +class HtmlDiffFileWriter: + + def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: + self.target_folder = target_folder + self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) + + def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: + file_relative = file.relative_to(pathlib.Path.cwd()) + out_file_name: str = str(file_relative).replace('/', '_') + '.html' + with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: + out_file.write(self.differ.make_file(src, target, context=False)) + + +def uncrustify_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], + write_diff_file: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] + ) -> List[utils.ReportData]: + result: List[utils.ReportData] = [] + for src_file, uncrustify_file in files: + with open(src_file, encoding="utf-8") as src: + src_lines = src.readlines() + with open(uncrustify_file, encoding="utf-8") as fixed: + fixed_lines = fixed.readlines() + + issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) + if issues and write_diff_file: + write_diff_file(src_file, src_lines, fixed_lines) + result.extend(issues) + return result def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: @@ -57,5 +99,69 @@ def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: return wrapcolumn, tabsize +def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: + result = [] + matching_blocks: List[difflib.Match] = \ + difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() + previous_match = matching_blocks[0] + for match in matching_blocks[1:]: + block = _get_mismatching_block(previous_match, match, src, target) + previous_match = match + if not block: + continue + line, before, after = block + path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) + message = _get_issue_message(before, after) + result.append(utils.ReportData( + symbol="Uncrustify Code Style issue", + message=message, + path=str(path), + line=line + )) + + return result + + +def _get_issue_message(before: str, after: str) -> str: + # The maximum number of lines to write separate comments for + # If exceeded, summarized comment will be provided instead + max_lines = 11 + diff_size = len(before.splitlines()) + if diff_size > max_lines: + message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ + f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" + else: + # Message with before&after + message = f"\nOriginal code:\n```diff\n{before}```\n" + \ + f"Uncrustify generated code:\n```diff\n{after}```\n" + return message + + +def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] + match: difflib.Match, + src: List[str], target: List[str] + ) -> Optional[Tuple[int, str, str]]: + previous_match_end_in_src = previous_match.a + previous_match.size + previous_match_end_in_target = previous_match.b + previous_match.size + match_start_in_src = match.a + match_start_in_target = match.b + if previous_match_end_in_src == match_start_in_src: + return None + line = match_start_in_src + before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) + after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) + return line, before, after + + +def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: + return _replace_invisible_symbols(''.join(lines[start: end])) + + +def _replace_invisible_symbols(line: str) -> str: + for old_str, new_str in zip([" ", "\t", "\n"], ["\u00b7", "\u2192\u2192\u2192\u2192", "\u2193\u000a"]): + line = line.replace(old_str, new_str) + return line + + if __name__ == "__main__": - main() + main() # pylint: disable=no-value-for-parameter # see https://github.com/PyCQA/pylint/issues/259 diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index 7d921709..a6bfd452 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -1,12 +1,11 @@ +import json +import sys import argparse import glob -import json -import os import pathlib import subprocess -import sys -from typing import Any, Callable, List, Optional, Tuple, Set, Iterable +from typing import Any, Callable, List, Optional, Tuple, Set from typing_extensions import TypedDict from universum.lib.ci_exception import CiException @@ -20,13 +19,6 @@ def __init__(self, code: int = 2, message: Optional[str] = None): self.message: Optional[str] = message -def create_parser(description: str, module_path: str) -> argparse.ArgumentParser: - module_name, _ = os.path.splitext(os.path.basename(module_path)) - - prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {__package__}.{module_name}" - return argparse.ArgumentParser(prog=prog, description=description) - - def analyzer(parser: argparse.ArgumentParser): """ Wraps the analyzer specific data and adds common protocol information: @@ -37,7 +29,6 @@ def analyzer(parser: argparse.ArgumentParser): :param parser: Definition of analyzer custom arguments :return: Wrapped analyzer with common reporting behaviour """ - def internal(func: Callable[[argparse.Namespace], List[ReportData]]) -> Callable[[], List[ReportData]]: def wrapper() -> List[ReportData]: add_files_argument(parser) @@ -84,7 +75,6 @@ def sys_exit(func: Callable[[], Any]) -> Callable[[], None]: >>> wrap_system_exit(sys_exit(_raise_custom)) 3 """ - def wrapper() -> None: exit_code: int try: @@ -156,16 +146,6 @@ def report_to_file(issues: List[ReportData], json_file: Optional[str] = None) -> sys.stdout.write(issues_json) -def normalize_path(file: str) -> pathlib.Path: +def normalize(file: str) -> pathlib.Path: file_path = pathlib.Path(file) return file_path if file_path.is_absolute() else pathlib.Path.cwd().joinpath(file_path) - - -def get_files_with_absolute_paths(settings: argparse.Namespace) -> Iterable[Tuple[pathlib.Path, - pathlib.Path, - pathlib.Path]]: - for src_file in settings.file_list: - src_file_absolute = normalize_path(src_file) - src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) - target_file_absolute: pathlib.Path = settings.target_folder.joinpath(src_file_relative) - yield src_file_absolute, target_file_absolute, src_file_relative diff --git a/universum/configuration_support.py b/universum/configuration_support.py index efd76872..bf798af8 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -355,7 +355,7 @@ def get(self, key: str, default: Any = None) -> Any: ... warnings.simplefilter("always") ... f() ... return w - >>> step = Step(name='foo', my_var='bar', t1=None, t2=False) + >>> step = Step(name='foo', my_var='bar') >>> do_and_get_warnings(lambda : step.get('name', 'test')) # doctest: +ELLIPSIS [] @@ -367,16 +367,12 @@ def get(self, key: str, default: Any = None) -> Any: 'test' >>> step.get('command', 'test') 'test' - >>> step.get('t1') is None - True - >>> step.get('t2') - False """ result = self._extras.get(key) - if result is not None: # for custom fields there is a distinction between None and falsy values + if result: return result result = self.__dict__.get(key) - if result: # non-custom fields initialized with falsy values + if result: warn("Using legacy API to access configuration values. Please use var." + key + " instead.") return result return default diff --git a/universum/modules/code_report_collector.py b/universum/modules/code_report_collector.py index edb2e795..e558814c 100644 --- a/universum/modules/code_report_collector.py +++ b/universum/modules/code_report_collector.py @@ -92,6 +92,9 @@ def report_code_report_results(self) -> None: if text: report = json.loads(text) + json_file: TextIO = self.artifacts.create_text_file("Static_analysis_report.json") + json_file.write(json.dumps(report, indent=4)) + issue_count: int if not report and report != []: self.out.log_error("There are no results in code report file. Something went wrong.") diff --git a/universum/modules/vcs/swarm.py b/universum/modules/vcs/swarm.py index ca671301..53eb8e46 100644 --- a/universum/modules/vcs/swarm.py +++ b/universum/modules/vcs/swarm.py @@ -9,7 +9,7 @@ from ...lib.ci_exception import CiException from ...lib.gravity import Dependency -urllib3.disable_warnings(urllib3.exceptions.InsecurePlatformWarning) # type: ignore +urllib3.disable_warnings((urllib3.exceptions.InsecurePlatformWarning, urllib3.exceptions.SNIMissingWarning)) # type: ignore __all__ = [ "Swarm" From 99227aaec14a74e62496561e822b8732d4ae5650 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Fri, 26 May 2023 17:43:21 +0300 Subject: [PATCH 16/25] feat(configuration_support): change conditional branch steps type to Configuration (#808) --- doc/configuring.rst | 12 +++++------ tests/test_conditional_steps.py | 4 ++-- universum/configuration_support.py | 18 ++++++++--------- universum/modules/artifact_collector.py | 27 +++++++++++-------------- universum/modules/structure_handler.py | 4 ++-- 5 files changed, 31 insertions(+), 34 deletions(-) diff --git a/doc/configuring.rst b/doc/configuring.rst index ce0b5962..551ebe8d 100644 --- a/doc/configuring.rst +++ b/doc/configuring.rst @@ -441,9 +441,9 @@ If any of them is missing or not set in current environment, the step will be ex Conditional steps --------------------- -Conditional step is a :class:`Step` object, that has ``if_succeeded`` or ``if_failed`` parameters with other steps assigned. -If the conditional step succeeds, then the step from the ``if_succeeded`` parameter will be executed. -If the conditional step fails, the step from the ``if_failed`` parameter will be executed instead. +Conditional step is a :class:`Step` object, that has ``if_succeeded`` or ``if_failed`` parameters with other :class:`Configuration` objects assigned. +If the conditional step succeeds, then the Configuration from the ``if_succeeded`` parameter will be executed. +If the conditional step fails, then the Configuration from the ``if_failed`` parameter will be executed instead. Configuration example: @@ -451,8 +451,8 @@ Configuration example: from universum.configuration_support import Configuration, Step - true_branch_step = Step(name="Positive branch step", command=["ls"]) - false_branch_step = Step(name="Negative branch step", command=["pwd"]) + true_branch_step = Configuration([Step(name="Positive branch step", command=["ls"])]) + false_branch_step = Configuration([Step(name="Negative branch step", command=["pwd"])]) conditional_step = Step(name="Conditional step", command=["./script.sh"], if_succeeded=true_branch_step, if_failed=false_branch_step) @@ -502,7 +502,7 @@ In general, conditional steps behave as any other regular steps, but here are so * Will always be marked as successful in the log * TeamCity tag will not be set for the conditional step * Branch steps - * Only one branch step will be executed + * Only one branch Configuration will be executed * Both branches' artifacts will be checked for existence before the steps execution * Artifacts collection or any other side-effects will not be triggered for non-executed branch step * If chosen branch step is not set, nothing will happen. diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index a7316370..d27f1a0d 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -98,9 +98,9 @@ def _build_step_command(files_to_create, exit_code) -> List[str]: return ["bash", "-c", ";".join(commands)] def _write_config_file(self, steps_info) -> None: - true_branch_step: str = f"Step(**{str(steps_info.true_branch_step)})" \ + true_branch_step: str = f"Configuration([Step(**{str(steps_info.true_branch_step)})])" \ if steps_info.true_branch_step else "None" - false_branch_step: str = f"Step(**{str(steps_info.false_branch_step)})" \ + false_branch_step: str = f"Configuration([Step(**{str(steps_info.false_branch_step)})])" \ if steps_info.false_branch_step else "None" config_lines: List[str] = [ "from universum.configuration_support import Configuration, Step", diff --git a/universum/configuration_support.py b/universum/configuration_support.py index efd76872..43831a8c 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -125,11 +125,11 @@ class Step: fail_tag A tag used to mark failed TeamCity builds. See `pass_tag` for details. Not applicable for conditional steps. if_succeeded - Another step, that will be executed in case of this step will succeed. Having this parameter non-None will - make the current step conditional. + Another Configuration, that will be executed in case of this step will succeed. + Having this parameter non-None will make the current step conditional. if_failed - Another step, that will be executed in case of this step will fail. Having this parameter non-None will - make the current step conditional. + Another Configuration, that will be executed in case of this step will fail. + Having this parameter non-None will make the current step conditional. Each parameter is optional, and is substituted with a falsy value, if omitted. @@ -182,8 +182,8 @@ def __init__(self, pass_tag: str = '', fail_tag: str = '', if_env_set: str = '', - if_succeeded = None, - if_failed = None, + if_succeeded: Optional['Configuration'] = None, + if_failed: Optional['Configuration'] = None, **kwargs) -> None: self.name: str = name self.directory: str = directory @@ -199,9 +199,9 @@ def __init__(self, self.pass_tag: str = pass_tag self.fail_tag: str = fail_tag self.if_env_set: str = if_env_set - self.if_succeeded = if_succeeded - self.if_failed = if_failed - self.is_conditional = self.if_succeeded or self.if_failed + self.if_succeeded: Optional['Configuration'] = if_succeeded + self.if_failed: Optional['Configuration'] = if_failed + self.is_conditional: bool = bool(self.if_succeeded or self.if_failed) self.children: Optional['Configuration'] = None self._extras: Dict[str, str] = {} for key, value in kwargs.items(): diff --git a/universum/modules/artifact_collector.py b/universum/modules/artifact_collector.py index c6748d23..1a1c5373 100644 --- a/universum/modules/artifact_collector.py +++ b/universum/modules/artifact_collector.py @@ -173,24 +173,21 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin with self.structure.block(block_name=name, pass_errors=True): self.preprocess_artifact_list(artifact_list, ignore_existing_artifacts) - def get_conditional_step_branches_artifacts(self, step: Step) -> List[ArtifactInfo]: - artifacts: List[Optional[ArtifactInfo]] = [ - self.get_config_artifact_if_exists(step.if_succeeded), - self.get_config_artifact_if_exists(step.if_failed), - self.get_config_artifact_if_exists(step.if_succeeded, is_report_artifact=True), - self.get_config_artifact_if_exists(step.if_failed, is_report_artifact=True) - ] + def get_conditional_step_branches_artifacts(self, conditional_step: Step) -> List[ArtifactInfo]: + steps_to_process: List[Step] = [] + if conditional_step.if_succeeded: + steps_to_process.extend(list(conditional_step.if_succeeded.all())) + if conditional_step.if_failed: + steps_to_process.extend(list(conditional_step.if_failed.all())) + + artifacts: List[Optional[ArtifactInfo]] = [] + for step in steps_to_process: + artifacts.append(self.get_config_artifact(step)) + artifacts.append(self.get_config_artifact(step, is_report_artifact=True)) + defined_artifacts: List[ArtifactInfo] = [artifact for artifact in artifacts if artifact] return defined_artifacts - def get_config_artifact_if_exists(self, step: Step, is_report_artifact: bool = False) -> Optional[ArtifactInfo]: - if not step: - return None - artifact: str = step.report_artifacts if is_report_artifact else step.artifacts - if not artifact: - return None - return self.get_config_artifact(step, is_report_artifact) - def get_config_artifact(self, step: Step, is_report_artifact: bool = False) -> ArtifactInfo: artifact: str = step.report_artifacts if is_report_artifact else step.artifacts path: str = utils.parse_path(artifact, self.settings.project_root) diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index 8a81cffb..733699a1 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -247,11 +247,11 @@ def execute_steps_recursively(self, parent: Step, elif child.is_conditional: conditional_step_succeeded: bool = self.process_one_step(merged_item, step_executor, skip_execution=False) - step_to_execute: Step = merged_item.if_succeeded if conditional_step_succeeded \ + step_to_execute: Optional[Configuration] = merged_item.if_succeeded if conditional_step_succeeded \ else merged_item.if_failed if step_to_execute: # branch step can be not set return self.execute_steps_recursively(parent=Step(), - children=Configuration([step_to_execute]), + children=step_to_execute, step_executor=step_executor, skip_execution=False) current_step_failed = False # conditional step should be always successful From d45c5df26b07ead9f575bbf6df4a6543f86c7194 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Mon, 29 May 2023 13:33:33 +0300 Subject: [PATCH 17/25] test(configuration_support): test Step->Configuration update for conditional step branches (#809) --- tests/test_conditional_steps.py | 110 +++++++++++++++++--------------- 1 file changed, 57 insertions(+), 53 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index d27f1a0d..647de3e0 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -13,8 +13,8 @@ class StepsInfo: conditional_step: Step - true_branch_step: Optional[Step] - false_branch_step: Optional[Step] + true_branch_steps: Optional[List[Step]] + false_branch_steps: Optional[List[Step]] is_conditional_step_passed: bool @@ -28,8 +28,8 @@ def build_conditional_steps_info(self, is_conditional_step_passed: bool) -> Step steps_info: StepsInfo = StepsInfo() steps_info.conditional_step = self._build_conditional_step(is_conditional_step_passed) - steps_info.true_branch_step = self._build_true_branch_step() - steps_info.false_branch_step = self._build_false_branch_step() + steps_info.true_branch_steps = self._build_true_branch_steps() + steps_info.false_branch_steps = self._build_false_branch_steps() steps_info.is_conditional_step_passed = is_conditional_step_passed return steps_info @@ -67,27 +67,24 @@ def _build_conditional_step(self, is_conditional_step_passed: bool) -> Step: artifacts=conditional_step_artifact, report_artifacts=conditional_step_report_artifact) - def _build_true_branch_step(self) -> Step: - true_branch_step_name: str = "true_branch" - true_branch_step_artifact: str = f"{true_branch_step_name}_artifact" - true_branch_step_report_artifact: str = f"{true_branch_step_name}_report_artifact" - return Step(name=true_branch_step_name, - command=self._build_step_command( - files_to_create=[true_branch_step_artifact, true_branch_step_report_artifact], - exit_code=0), - artifacts=true_branch_step_artifact, - report_artifacts=true_branch_step_report_artifact) - - def _build_false_branch_step(self) -> Step: - false_branch_step_name: str = "false_branch" - false_branch_step_artifact: str = f"{false_branch_step_name}_artifact" - false_branch_step_report_artifact: str = f"{false_branch_step_name}_report_artifact" - return Step(name=false_branch_step_name, - command=self._build_step_command( - files_to_create=[false_branch_step_artifact, false_branch_step_report_artifact], - exit_code=0), - artifacts=false_branch_step_artifact, - report_artifacts=false_branch_step_report_artifact) + def _build_true_branch_steps(self) -> List[Step]: + return self._build_branch_steps(["true_branch", "true_branch_dependent1", "true_branch_dependent2"]) + + def _build_false_branch_steps(self) -> List[Step]: + return self._build_branch_steps(["false_branch", "false_branch_dependent"]) + + def _build_branch_steps(self, step_names: List[str]) -> List[Step]: + steps = [] + for step_name in step_names: + step_artifact: str = f"{step_name}_artifact" + step_report_artifact: str = f"{step_name}_report_artifact" + steps.append(Step(name=step_name, + command=self._build_step_command( + files_to_create=[step_artifact, step_report_artifact], + exit_code=0), + artifacts=step_artifact, + report_artifacts=step_report_artifact)) + return steps @staticmethod def _build_step_command(files_to_create, exit_code) -> List[str]: @@ -98,20 +95,18 @@ def _build_step_command(files_to_create, exit_code) -> List[str]: return ["bash", "-c", ";".join(commands)] def _write_config_file(self, steps_info) -> None: - true_branch_step: str = f"Configuration([Step(**{str(steps_info.true_branch_step)})])" \ - if steps_info.true_branch_step else "None" - false_branch_step: str = f"Configuration([Step(**{str(steps_info.false_branch_step)})])" \ - if steps_info.false_branch_step else "None" + true_branch_config = self._build_branch_configuration_string(steps_info.true_branch_steps) + false_branch_config = self._build_branch_configuration_string(steps_info.false_branch_steps) config_lines: List[str] = [ "from universum.configuration_support import Configuration, Step", - f"true_branch_step = {true_branch_step}", - f"false_branch_step = {false_branch_step}", + f"true_branch_config = {true_branch_config}", + f"false_branch_config = {false_branch_config}", f"conditional_step = Step(**{str(steps_info.conditional_step)})", # `true/false_branch_steps` should be Python objects from this script "conditional_step.is_conditional = True", - "conditional_step.if_succeeded = true_branch_step", - "conditional_step.if_failed = false_branch_step", + "conditional_step.if_succeeded = true_branch_config", + "conditional_step.if_failed = false_branch_config", "configs = Configuration([conditional_step])" ] @@ -119,26 +114,35 @@ def _write_config_file(self, steps_info) -> None: self.configs_file.write_text(config, "utf-8") + @staticmethod + def _build_branch_configuration_string(steps: Optional[List[Step]]) -> str: + if not steps: + return "None" + steps_strings: List[str] = [f"Step(**{str(step)})" for step in steps] + steps_list_string: str = ", ".join(steps_strings) + return f"Configuration([{steps_list_string}])" + def _check_conditional_step_artifacts_present(self, steps_info: StepsInfo) -> None: conditional_step: Step = steps_info.conditional_step - self._check_artifacts_presence(conditional_step, is_presence_expected=True) + self._check_artifacts_presence([conditional_step], is_presence_expected=True) def _check_executed_step_artifacts_present(self, steps_info: StepsInfo) -> None: - executed_step: Optional[Step] = steps_info.true_branch_step if steps_info.is_conditional_step_passed \ - else steps_info.false_branch_step - self._check_artifacts_presence(executed_step, is_presence_expected=True) + executed_steps: Optional[List[Step]] = steps_info.true_branch_steps \ + if steps_info.is_conditional_step_passed else steps_info.false_branch_steps + self._check_artifacts_presence(executed_steps, is_presence_expected=True) def _check_not_executed_step_artifacts_absent(self, steps_info: StepsInfo) -> None: - not_executed_step: Optional[Step] = steps_info.false_branch_step if steps_info.is_conditional_step_passed \ - else steps_info.true_branch_step - self._check_artifacts_presence(not_executed_step, is_presence_expected=False) + not_executed_steps: Optional[List[Step]] = steps_info.false_branch_steps \ + if steps_info.is_conditional_step_passed else steps_info.true_branch_steps + self._check_artifacts_presence(not_executed_steps, is_presence_expected=False) - def _check_artifacts_presence(self, step: Optional[Step], is_presence_expected: bool): - if not step: # branch step can be not set + def _check_artifacts_presence(self, steps: Optional[List[Step]], is_presence_expected: bool): + if not steps: # branch configuration can be not set return - for artifact in [step.artifacts, step.report_artifacts]: - artifact_path: pathlib.Path = self.artifact_dir / artifact - assert artifact_path.exists() == is_presence_expected + for step in steps: + for artifact in [step.artifacts, step.report_artifacts]: + artifact_path: pathlib.Path = self.artifact_dir / artifact + assert artifact_path.exists() == is_presence_expected @pytest.fixture() @@ -162,23 +166,23 @@ def test_conditional_false_branch(test_env: ConditionalStepsTestEnv) -> None: def test_same_artifact(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) - assert steps_info.true_branch_step - steps_info.true_branch_step.command = steps_info.conditional_step.command - steps_info.true_branch_step.artifacts = steps_info.conditional_step.artifacts - steps_info.true_branch_step.report_artifacts = steps_info.conditional_step.report_artifacts + assert steps_info.true_branch_steps + steps_info.true_branch_steps[0].command = steps_info.conditional_step.command + steps_info.true_branch_steps[0].artifacts = steps_info.conditional_step.artifacts + steps_info.true_branch_steps[0].report_artifacts = steps_info.conditional_step.report_artifacts test_env.check_success(steps_info) def test_true_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) - steps_info.true_branch_step = None + steps_info.true_branch_steps = None test_env.check_success(steps_info) def test_false_branch_chosen_but_absent(test_env: ConditionalStepsTestEnv) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) - steps_info.false_branch_step = None + steps_info.false_branch_steps = None test_env.check_success(steps_info) @@ -191,8 +195,8 @@ def test_prebuild_clean(test_env: ConditionalStepsTestEnv, is_report_artifact: bool) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) - assert steps_info.true_branch_step - step: Step = steps_info.true_branch_step if is_branch_step else steps_info.conditional_step + assert steps_info.true_branch_steps + step: Step = steps_info.true_branch_steps[0] if is_branch_step else steps_info.conditional_step step.artifact_prebuild_clean = prebuild_clean test_env.create_step_artifact(step, is_report_artifact) From 6f8e147bc62d86fa34007ed55caed89b5b3a0798 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Wed, 7 Jun 2023 06:21:58 +0300 Subject: [PATCH 18/25] fix(launcher): exit with error if condition step has children (#812) --- tests/test_conditional_steps.py | 20 ++++++++++++++++---- universum/modules/launcher.py | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 647de3e0..b0422fbf 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -6,7 +6,7 @@ import pytest -from universum.configuration_support import Step +from universum.configuration_support import Step, Configuration from .utils import LocalTestEnvironment @@ -95,18 +95,22 @@ def _build_step_command(files_to_create, exit_code) -> List[str]: return ["bash", "-c", ";".join(commands)] def _write_config_file(self, steps_info) -> None: - true_branch_config = self._build_branch_configuration_string(steps_info.true_branch_steps) - false_branch_config = self._build_branch_configuration_string(steps_info.false_branch_steps) + true_branch_config = self._build_configuration_string(steps_info.true_branch_steps) + false_branch_config = self._build_configuration_string(steps_info.false_branch_steps) + conditional_children_config = self._build_configuration_string(steps_info.conditional_step.children) + steps_info.conditional_step.children = None # will be set in config text config_lines: List[str] = [ "from universum.configuration_support import Configuration, Step", f"true_branch_config = {true_branch_config}", f"false_branch_config = {false_branch_config}", + f"conditional_children_config = {conditional_children_config}", f"conditional_step = Step(**{str(steps_info.conditional_step)})", # `true/false_branch_steps` should be Python objects from this script "conditional_step.is_conditional = True", "conditional_step.if_succeeded = true_branch_config", "conditional_step.if_failed = false_branch_config", + "conditional_step.children = conditional_children_config", "configs = Configuration([conditional_step])" ] @@ -115,7 +119,7 @@ def _write_config_file(self, steps_info) -> None: self.configs_file.write_text(config, "utf-8") @staticmethod - def _build_branch_configuration_string(steps: Optional[List[Step]]) -> str: + def _build_configuration_string(steps: Optional[List[Step]]) -> str: if not steps: return "None" steps_strings: List[str] = [f"Step(**{str(step)})" for step in steps] @@ -204,3 +208,11 @@ def test_prebuild_clean(test_env: ConditionalStepsTestEnv, test_env.check_success(steps_info) else: test_env.check_fail(steps_info) + + +# TODO: implement support of conditional step with children +# https://github.com/Samsung/Universum/issues/709 +def test_conditional_step_with_children(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) + steps_info.conditional_step.children = Configuration([Step(name=" dummy child 1"), Step(name=" dummy child 2")]) + test_env.check_fail(steps_info) diff --git a/universum/modules/launcher.py b/universum/modules/launcher.py index fe0b974e..af68c4ea 100644 --- a/universum/modules/launcher.py +++ b/universum/modules/launcher.py @@ -414,6 +414,9 @@ def process_project_configs(self) -> configuration_support.Configuration: f"\tExclude patterns: {self.exclude_patterns}" raise CriticalCiException(text) + if self._is_conditional_step_with_children_present(self.project_config): + raise CriticalCiException("Conditional step doesn't support children configuration") + return self.project_config def create_process(self, item: configuration_support.Step) -> RunningStep: @@ -436,3 +439,19 @@ def launch_custom_configs(self, custom_configs: configuration_support.Configurat def launch_project(self) -> None: self.reporter.add_block_to_report(self.structure.get_current_block()) self.structure.execute_step_structure(self.project_config, self.create_process) + + # TODO: implement support of conditional step with children + # https://github.com/Samsung/Universum/issues/709 + @staticmethod + def _is_conditional_step_with_children_present( + configuration: Optional[configuration_support.Configuration]) -> bool: + if not configuration: + return False + for step in configuration.configs: + if step.is_conditional and step.children: + return True + if Launcher._is_conditional_step_with_children_present(step.children) or \ + Launcher._is_conditional_step_with_children_present(step.if_succeeded) or \ + Launcher._is_conditional_step_with_children_present(step.if_failed): + return True + return False From d4b49328e22639cc8493f2dd5bd00fce041c4ee5 Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Wed, 14 Jun 2023 15:54:21 +0300 Subject: [PATCH 19/25] update(doc): increase version --- universum/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/universum/__init__.py b/universum/__init__.py index efd4d5e5..e1d5d322 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.16" +__version__ = "0.19.17" From 7ed4cc6f40a7eab69cfe1b2a61e41ab1ef4fb1e3 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 15 Jun 2023 11:28:58 +0300 Subject: [PATCH 20/25] fix(structure_handler): execute steps after conditinal step (#816) --- tests/test_conditional_steps.py | 20 ++++++++++++++++---- universum/modules/structure_handler.py | 8 ++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index b0422fbf..06ca9c61 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -15,6 +15,7 @@ class StepsInfo: conditional_step: Step true_branch_steps: Optional[List[Step]] false_branch_steps: Optional[List[Step]] + steps_after_conditional: List[Step] is_conditional_step_passed: bool @@ -30,6 +31,7 @@ def build_conditional_steps_info(self, is_conditional_step_passed: bool) -> Step steps_info.conditional_step = self._build_conditional_step(is_conditional_step_passed) steps_info.true_branch_steps = self._build_true_branch_steps() steps_info.false_branch_steps = self._build_false_branch_steps() + steps_info.steps_after_conditional = self._build_steps_after_conditional() steps_info.is_conditional_step_passed = is_conditional_step_passed return steps_info @@ -50,6 +52,7 @@ def check_success(self, steps_info: StepsInfo) -> None: self._check_conditional_step_artifacts_present(steps_info) self._check_executed_step_artifacts_present(steps_info) self._check_not_executed_step_artifacts_absent(steps_info) + self._check_steps_after_conditional_artifacts_present(steps_info) def check_fail(self, steps_info: StepsInfo) -> None: self._write_config_file(steps_info) @@ -68,12 +71,15 @@ def _build_conditional_step(self, is_conditional_step_passed: bool) -> Step: report_artifacts=conditional_step_report_artifact) def _build_true_branch_steps(self) -> List[Step]: - return self._build_branch_steps(["true_branch", "true_branch_dependent1", "true_branch_dependent2"]) + return self._build_steps_list(["true_branch", "true_branch_dependent1", "true_branch_dependent2"]) def _build_false_branch_steps(self) -> List[Step]: - return self._build_branch_steps(["false_branch", "false_branch_dependent"]) + return self._build_steps_list(["false_branch", "false_branch_dependent"]) - def _build_branch_steps(self, step_names: List[str]) -> List[Step]: + def _build_steps_after_conditional(self) -> List[Step]: + return self._build_steps_list(["step_after_conditional"]) + + def _build_steps_list(self, step_names: List[str]) -> List[Step]: steps = [] for step_name in step_names: step_artifact: str = f"{step_name}_artifact" @@ -98,6 +104,7 @@ def _write_config_file(self, steps_info) -> None: true_branch_config = self._build_configuration_string(steps_info.true_branch_steps) false_branch_config = self._build_configuration_string(steps_info.false_branch_steps) conditional_children_config = self._build_configuration_string(steps_info.conditional_step.children) + config_after_conditional = self._build_configuration_string(steps_info.steps_after_conditional) steps_info.conditional_step.children = None # will be set in config text config_lines: List[str] = [ "from universum.configuration_support import Configuration, Step", @@ -105,6 +112,7 @@ def _write_config_file(self, steps_info) -> None: f"false_branch_config = {false_branch_config}", f"conditional_children_config = {conditional_children_config}", f"conditional_step = Step(**{str(steps_info.conditional_step)})", + f"config_after_conditional = {config_after_conditional}", # `true/false_branch_steps` should be Python objects from this script "conditional_step.is_conditional = True", @@ -112,7 +120,7 @@ def _write_config_file(self, steps_info) -> None: "conditional_step.if_failed = false_branch_config", "conditional_step.children = conditional_children_config", - "configs = Configuration([conditional_step])" + "configs = Configuration([conditional_step]) + config_after_conditional" ] config: str = "\n".join(config_lines) @@ -140,6 +148,10 @@ def _check_not_executed_step_artifacts_absent(self, steps_info: StepsInfo) -> No if steps_info.is_conditional_step_passed else steps_info.true_branch_steps self._check_artifacts_presence(not_executed_steps, is_presence_expected=False) + def _check_steps_after_conditional_artifacts_present(self, steps_info: StepsInfo) -> None: + steps_after_conditional: List[Step] = steps_info.steps_after_conditional + self._check_artifacts_presence(steps_after_conditional, is_presence_expected=True) + def _check_artifacts_presence(self, steps: Optional[List[Step]], is_presence_expected: bool): if not steps: # branch configuration can be not set return diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index 733699a1..8d240290 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -250,10 +250,10 @@ def execute_steps_recursively(self, parent: Step, step_to_execute: Optional[Configuration] = merged_item.if_succeeded if conditional_step_succeeded \ else merged_item.if_failed if step_to_execute: # branch step can be not set - return self.execute_steps_recursively(parent=Step(), - children=step_to_execute, - step_executor=step_executor, - skip_execution=False) + self.execute_steps_recursively(parent=Step(), + children=step_to_execute, + step_executor=step_executor, + skip_execution=False) current_step_failed = False # conditional step should be always successful else: if merged_item.finish_background and self.active_background_steps: From 679d825ca68e674c17a96c5e677b8a3ae79d02fc Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Thu, 15 Jun 2023 11:57:11 +0300 Subject: [PATCH 21/25] feat(workflow): switch to GitHub Actions VCS type --- .github/workflows/precommit-check.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/precommit-check.yml b/.github/workflows/precommit-check.yml index 1700d7eb..087c9fb4 100644 --- a/.github/workflows/precommit-check.yml +++ b/.github/workflows/precommit-check.yml @@ -1,5 +1,7 @@ name: Universum check -on: push +on: + pull_request: + types: [opened, synchronize] jobs: universum_check: @@ -19,9 +21,10 @@ jobs: run: python -u -m universum --fail-unsuccessful - --vcs-type=git - --git-repo $GITHUB_SERVER_URL/$GITHUB_REPOSITORY - --git-refspec $GITHUB_REF_NAME + --vcs-type="ghactions" + --ghactions-payload="@${{ github.event_path }}" + --ghactions-token="${{ secrets.GITHUB_TOKEN }}" + --report-to-review --no-archive --no-diff From 452abeb905f67019b95bafd3d77a3151da75742d Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Tue, 27 Jun 2023 11:14:42 +0300 Subject: [PATCH 22/25] =?UTF-8?q?feat(launcher):=20explicit=20warning=20ab?= =?UTF-8?q?out=20'critical'=20flag=20ignoring=20on=20co=E2=80=A6=20(#817)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_conditional_steps.py | 13 +++++++++++-- universum/modules/launcher.py | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 06ca9c61..2d402a91 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -24,6 +24,7 @@ class ConditionalStepsTestEnv(LocalTestEnvironment): def __init__(self, tmp_path: pathlib.Path, capsys: pytest.CaptureFixture) -> None: super().__init__(tmp_path, "main") self.capsys = capsys + self.captured_out: pytest.CaptureResult[AnyStr] = None def build_conditional_steps_info(self, is_conditional_step_passed: bool) -> StepsInfo: steps_info: StepsInfo = StepsInfo() @@ -45,9 +46,9 @@ def check_success(self, steps_info: StepsInfo) -> None: self._write_config_file(steps_info) self.run() - captured: pytest.CaptureResult[AnyStr] = self.capsys.readouterr() + self.captured_out = self.capsys.readouterr() conditional_succeeded_regexp: str = r"\] conditional.*Success.*\| 5\.2" - assert re.search(conditional_succeeded_regexp, captured.out, re.DOTALL) + assert re.search(conditional_succeeded_regexp, self.captured_out.out, re.DOTALL) self._check_conditional_step_artifacts_present(steps_info) self._check_executed_step_artifacts_present(steps_info) @@ -228,3 +229,11 @@ def test_conditional_step_with_children(test_env: ConditionalStepsTestEnv) -> No steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) steps_info.conditional_step.children = Configuration([Step(name=" dummy child 1"), Step(name=" dummy child 2")]) test_env.check_fail(steps_info) + + +def test_conditional_step_critical(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) + steps_info.conditional_step.critical = True + test_env.check_success(steps_info) + assert test_env.captured_out.out + assert "WARNING" in test_env.captured_out.out diff --git a/universum/modules/launcher.py b/universum/modules/launcher.py index af68c4ea..c195c14f 100644 --- a/universum/modules/launcher.py +++ b/universum/modules/launcher.py @@ -416,6 +416,7 @@ def process_project_configs(self) -> configuration_support.Configuration: if self._is_conditional_step_with_children_present(self.project_config): raise CriticalCiException("Conditional step doesn't support children configuration") + self._warn_if_critical_conditional_steps_present(self.project_config) return self.project_config @@ -455,3 +456,27 @@ def _is_conditional_step_with_children_present( Launcher._is_conditional_step_with_children_present(step.if_failed): return True return False + + def _warn_if_critical_conditional_steps_present( + self, configuration: Optional[configuration_support.Configuration]) -> None: + step_names: List[str] = Launcher._get_critical_conditional_step_names_recursively(configuration) + if not step_names: + return + step_names = [f'\t* "{name}"' for name in step_names] + step_names_str: str = "\n".join(step_names) + self.out.log(f"WARNING: 'critical' flag will be ignored for conditional steps: \n{step_names_str}\n " + "Set it to the 'if_failed' branch step instead") + + @staticmethod + def _get_critical_conditional_step_names_recursively( + configuration: Optional[configuration_support.Configuration]) -> List[str]: + step_names: List[str] = [] + if not configuration: + return step_names + for step in configuration.configs: + if step.is_conditional and step.critical: + step_names.append(step.name) + step_names.extend(Launcher._get_critical_conditional_step_names_recursively(step.children)) + step_names.extend(Launcher._get_critical_conditional_step_names_recursively(step.if_succeeded)) + step_names.extend(Launcher._get_critical_conditional_step_names_recursively(step.if_failed)) + return step_names From 63ef9e9c66034f5525c66eec57bd21bd722045f4 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Fri, 25 Aug 2023 11:46:51 +0300 Subject: [PATCH 23/25] fix(configuration_support): raise explicit error on conditional steps addition (#819) --- tests/test_conditional_steps.py | 63 ++++++++++++++++++++++++++---- universum/configuration_support.py | 4 ++ universum/modules/launcher.py | 2 +- 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index 2d402a91..95d03361 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -223,17 +223,66 @@ def test_prebuild_clean(test_env: ConditionalStepsTestEnv, test_env.check_fail(steps_info) +def test_conditional_step_critical(test_env: ConditionalStepsTestEnv) -> None: + steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) + steps_info.conditional_step.critical = True + test_env.check_success(steps_info) + assert test_env.captured_out.out + assert "WARNING" in test_env.captured_out.out + + # TODO: implement support of conditional step with children # https://github.com/Samsung/Universum/issues/709 -def test_conditional_step_with_children(test_env: ConditionalStepsTestEnv) -> None: +def test_conditional_step_with_children(test_env: ConditionalStepsTestEnv, capsys: pytest.CaptureFixture) -> None: steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True) steps_info.conditional_step.children = Configuration([Step(name=" dummy child 1"), Step(name=" dummy child 2")]) test_env.check_fail(steps_info) + captured_out = capsys.readouterr() + assert "not supported" in captured_out.out -def test_conditional_step_critical(test_env: ConditionalStepsTestEnv) -> None: - steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False) - steps_info.conditional_step.critical = True - test_env.check_success(steps_info) - assert test_env.captured_out.out - assert "WARNING" in test_env.captured_out.out + +# TODO: implement support of conditional steps addition and multiplication +# https://github.com/Samsung/Universum/issues/709 +def test_add_step_to_conditional(test_env: ConditionalStepsTestEnv, capsys: pytest.CaptureFixture) -> None: + config_lines: List[str] = [ + "from universum.configuration_support import Configuration, Step", + "true_branch_config = Configuration([Step(name='true branch')])", + "conditional_step = Step(name='conditional', if_succeeded=true_branch_config)", + "another_step = Step(name='another step')", + "configs = Configuration([conditional_step + another_step])" + ] + config: str = "\n".join(config_lines) + test_env.configs_file.write_text(config, "utf-8") + test_env.run(expect_failure=True) + + captured_out = capsys.readouterr() + assert "not supported" in captured_out.out + + +# TODO: during implementation of the full conditional steps addition support, refactor this test to avoid direct usage +# of the ConditionalStepsTestEnv "internal" API (underscored methods) and config building duplication +# https://github.com/Samsung/Universum/issues/709 +def test_add_conditional_to_step(test_env: ConditionalStepsTestEnv, capsys: pytest.CaptureFixture) -> None: + # pylint: disable = protected-access + steps_info = test_env.build_conditional_steps_info(is_conditional_step_passed=True) + true_branch_config = test_env._build_configuration_string(steps_info.true_branch_steps) + config_lines: List[str] = [ + "from universum.configuration_support import Configuration, Step", + f"true_branch_config = {true_branch_config}", + f"conditional_step = Step(**{str(steps_info.conditional_step)})", + "another_step = Step(name='another step')", + + # `true/false_branch_steps` should be Python objects from this script + "conditional_step.is_conditional = True", + "conditional_step.if_succeeded = true_branch_config", + + "configs = Configuration([another_step + conditional_step])" + ] + config: str = "\n".join(config_lines) + test_env.configs_file.write_text(config, "utf-8") + test_env.run() + + test_env._check_conditional_step_artifacts_present(steps_info) + test_env._check_executed_step_artifacts_present(steps_info) + # pylint: enable = protected-access diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 43831a8c..3b731a7e 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -3,6 +3,7 @@ from warnings import warn import copy import os +from .lib.ci_exception import CriticalCiException __all__ = [ @@ -397,6 +398,9 @@ def __add__(self, other: 'Step') -> 'Step': >>> step2 + step1 {'name': 'barfoo', 'command': ['bar', 'foo'], 'critical': True, 'background': True, 'my_var1': 'barfoo', 'my_var2': 'baz'} """ + if self.is_conditional: + # TODO: https://github.com/Samsung/Universum/issues/709 + raise CriticalCiException("Conditional steps addition is not supported yet") return Step( name=self.name + other.name, command=self.command + other.command, diff --git a/universum/modules/launcher.py b/universum/modules/launcher.py index c195c14f..e72a35b4 100644 --- a/universum/modules/launcher.py +++ b/universum/modules/launcher.py @@ -415,7 +415,7 @@ def process_project_configs(self) -> configuration_support.Configuration: raise CriticalCiException(text) if self._is_conditional_step_with_children_present(self.project_config): - raise CriticalCiException("Conditional step doesn't support children configuration") + raise CriticalCiException("Conditional steps with child configurations are not supported") self._warn_if_critical_conditional_steps_present(self.project_config) return self.project_config From eb913283dcd9d6d6b767e26628384d31dd5d5246 Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Wed, 6 Sep 2023 10:41:01 +0300 Subject: [PATCH 24/25] fix(swarm): quickfix for incorrect '404' response Currently Swarm server returns '404' when adding comments to newly added files in review, causing to report malfunction. --- universum/modules/vcs/swarm.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/universum/modules/vcs/swarm.py b/universum/modules/vcs/swarm.py index ca671301..89917696 100644 --- a/universum/modules/vcs/swarm.py +++ b/universum/modules/vcs/swarm.py @@ -197,10 +197,11 @@ def code_report_to_review(self, report): abs_path = os.path.join(self.client_root, path) if abs_path in self.mappings_dict: for issue in issues: - self.post_comment(issue['message'], - filename=self.mappings_dict[abs_path], - line=issue['line'], - no_notification=True) + try: + self.post_comment(issue['message'], filename=self.mappings_dict[abs_path], + line=issue['line'], no_notification=True) + except CiException as e: + self.out.log_error(str(e)) def report_result(self, result, report_text=None, no_vote=False): # Opening links, sent by Swarm From 00a00f7cc3f49648de9ea0bfedb66110de73bd9a Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Wed, 6 Sep 2023 12:56:41 +0300 Subject: [PATCH 25/25] update(docs): update changelog to Universum 0.19.17 --- CHANGELOG.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ba610e1e..d08f83b6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,21 @@ Change log ========== +0.19.17 (2023-09-06) +-------------------- + +New features +~~~~~~~~~~~~ + +* **config:** add support of `conditional steps `__ + +Bug fixes +~~~~~~~~~ + +* **swarm:** quickfix for incorrect '404' response (Swarm server returning '404' + when adding comments to newly added files in review, causing report malfunction) + + 0.19.16 (2023-06-14) --------------------