From e0c602da2b554856d2da73dc1607018b9b4b90f2 Mon Sep 17 00:00:00 2001 From: RotemAmit Date: Thu, 16 Nov 2023 18:23:32 +0200 Subject: [PATCH] Generalize the mode option in pre-commit (#3765) * added get and set_properties, added to init * removed PreCommitModes * pre-commit updates * a small update in validate_format hook * added coverage_analyze and general_hook * added merge-coverage-report hook * pre-commit fixes * unit tests fixed * test_coverage_analyze_general_hook * pre-commit fixes * fixed conflicts * updated the call to GeneralHook * removed PreCommitModes from generic_docker_test.py * removed to_delete from hook init and updated a variable name * pre-commit fixes * updated the README and changelog * updates in dockerhook, removed generic hook, README update * pre-commit fixes,deleted general_hook file * cr fix * pre-commit fix * docker.py update * cr updates * cr updates * updated test__set_properties --- .pre-commit-hooks.yaml | 15 ++++ CHANGELOG.md | 3 + demisto_sdk/__main__.py | 3 - demisto_sdk/commands/common/constants.py | 4 - .../coverage_analyze/coverage_report.py | 4 +- demisto_sdk/commands/pre_commit/README.md | 28 +++--- .../commands/pre_commit/hooks/docker.py | 18 ++-- demisto_sdk/commands/pre_commit/hooks/hook.py | 35 +++----- demisto_sdk/commands/pre_commit/hooks/ruff.py | 18 ++-- .../commands/pre_commit/hooks/sourcery.py | 3 - .../pre_commit/hooks/validate_format.py | 17 ++-- .../commands/pre_commit/pre_commit_command.py | 50 +++++++---- .../pre_commit/tests/generic_docker_test.py | 22 ++--- .../pre_commit/tests/pre_commit_test.py | 66 +++++++++++--- .../run_unit_tests/unit_tests_runner.py | 75 ---------------- demisto_sdk/scripts/merge_coverage_report.py | 87 +++++++++++++++++++ pyproject.toml | 1 + 17 files changed, 262 insertions(+), 187 deletions(-) create mode 100644 demisto_sdk/scripts/merge_coverage_report.py diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 5508c6badb..b581900626 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -56,3 +56,18 @@ files: poetry.lock require_serial: true pass_filenames: false + +- id: coverage-analyze + name: coverage-analyze + entry: demisto-sdk coverage-analyze + description: Running demisto-sdk coverage-analyze and showing a coverage report. + language: python + pass_filenames: false + args: ["-i", ".coverage"] + +- id: merge-coverage-report + name: merge-coverage-report + entry: merge-coverage-report + language: python + require_serial: true + pass_filenames: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 09e28d5367..f7fec93ee9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ * Added a new service for file management, allowing to write files and read files from specific git branches, local file system, or from any remote api. * Added a new flag `--docker/--no-docker` to demisto-sdk pre-commit, in order to enable the option to run the pre-commit command without docker hooks. * Added support for xsoar, xsoar-saas and xsiam wrapper clients to ease the integration via their apis. +* Added the command demisto-sdk coverage-analyze to the pre-commit hooks. +* Updated merge_coverage_report to be a hook in the pre-commit. +* Updated the mode option to be free text. for more details see https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/pre_commit/README.md#modes * Added a new command **setup-env** to setup the environment for integrations and scripts in vs code IDE, XSOAR and XSIAM. * Fixed an issue where the SDK failed to retrieve docker hub token when there were temporary connection errors. * Internal: Added a welcome comment to contributions PRs. diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py index 1b73811596..01700c9545 100644 --- a/demisto_sdk/__main__.py +++ b/demisto_sdk/__main__.py @@ -24,7 +24,6 @@ ENV_DEMISTO_SDK_MARKETPLACE, FileType, MarketplaceVersions, - PreCommitModes, ) from demisto_sdk.commands.common.content_constant_paths import ( ALL_PACKS_DEPENDENCIES_DEFAULT_PATH, @@ -3398,7 +3397,6 @@ def update_content_graph( @click.option( "--mode", help="Special mode to run the pre-commit with", - type=click.Choice([mode.value for mode in list(PreCommitModes)]), ) @click.option( "-ut/--no-ut", @@ -3485,7 +3483,6 @@ def pre_commit( ): from demisto_sdk.commands.pre_commit.pre_commit_command import pre_commit_manager - mode = PreCommitModes(mode) if mode else None if file_paths and input: logger.info( "Both `--input` parameter and `file_paths` arguments were provided. Will use the `--input` parameter." diff --git a/demisto_sdk/commands/common/constants.py b/demisto_sdk/commands/common/constants.py index 37ee9ad9ff..82f85852bb 100644 --- a/demisto_sdk/commands/common/constants.py +++ b/demisto_sdk/commands/common/constants.py @@ -1934,7 +1934,3 @@ class ParameterType(Enum): class ImagesFolderNames(str, Enum): README_IMAGES = "readme_images" INTEGRATION_DESCRIPTION_IMAGES = "integration_description_images" - - -class PreCommitModes(str, Enum): - NIGHTLY = "nightly" diff --git a/demisto_sdk/commands/coverage_analyze/coverage_report.py b/demisto_sdk/commands/coverage_analyze/coverage_report.py index 03265b7f38..d1ef5a54ab 100644 --- a/demisto_sdk/commands/coverage_analyze/coverage_report.py +++ b/demisto_sdk/commands/coverage_analyze/coverage_report.py @@ -134,8 +134,8 @@ def coverage_diff_report(self) -> bool: min_cov = self.file_min_coverage(file_name) if min_cov > cov_precents: logger.error( - f"[red]Unit-tests for '{file_name}' must reach a coverage of at least {min_cov}% " - f"(currently: {cov_precents}%).[/red]" + f"[red]Unit-tests for '{file_name}' must reach a coverage of at least {min_cov} percent " + f"(currently: {cov_precents} percent).[/red]" ) coverage_ok = False return coverage_ok diff --git a/demisto_sdk/commands/pre_commit/README.md b/demisto_sdk/commands/pre_commit/README.md index 8bb242bd8c..142f6f76ab 100644 --- a/demisto_sdk/commands/pre_commit/README.md +++ b/demisto_sdk/commands/pre_commit/README.md @@ -18,6 +18,21 @@ It utilizes the [pre-commit](https://github.com/pre-commit/pre-commit) infrastru ### Automatically, as a git hook * Create a [git hook](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks) that calls `demisto-sdk pre-commit`. +## Modes +When different args set for the different modes are needed, for example, some rules should be excluded in the nightly build. +Any key can be set this way. +You can set this as follows. +```yaml + - id: sample-hook + args:nightly: ['--exclude', '123'] + args:mode1: ['--test', '123'] + args: ['This is the default argument'] + otherkey:nightly: hello + otherkey: world +``` +And call precommit as follows: `demisto-sdk pre-commit -a --mode nightly`. +Note, that it is possible to use any mode that you like, and have multiple modes for each hook, like in the example. + ## Steps ### External tools @@ -62,19 +77,6 @@ A docker hook must be under the `local` repo and it's `id` must end with `in-doc The `entry` key should be the command that should be run (eg. pylint, pytest). -#### Modes -If you need different args set for the different modes, for example some rules should be excluded in the nightly build. -Any key can be set this way. -You can set this as follows. -```yaml - - id: simple-in-docker - args:nightly: ['--exclude', '123'] - args: ['This is the default argument'] - otherkey:nightly: hello - otherkey: world -``` -And call precommit as follows: `demisto-sdk pre-commit -a --mode nightly` -Note, only "nightly" is currently supported. This will be free text soon. #### The config_file_arg key Often with commands we run in the docker we have a configuration file that is specified per Integration/Script. To configure this you can set the `config_file_arg` key as follows. The configuration file should be in the same directory as the code file. Here is an example with ruff. ```yaml diff --git a/demisto_sdk/commands/pre_commit/hooks/docker.py b/demisto_sdk/commands/pre_commit/hooks/docker.py index 77a5fe0866..1f513a7270 100644 --- a/demisto_sdk/commands/pre_commit/hooks/docker.py +++ b/demisto_sdk/commands/pre_commit/hooks/docker.py @@ -258,12 +258,13 @@ def get_new_hooks( Returns: All the hooks to be appended for this image """ - new_hook = { - "id": f"{self._get_property('id')}-{image}", - "name": f"{self._get_property('name')}-{image}", - "language": "docker_image", - "entry": f'--entrypoint {self._get_property("entry")} {get_environment_flag()} {dev_image}', - } + new_hook = deepcopy(self.base_hook) + new_hook["id"] = f"{new_hook.get('id')}-{image}" + new_hook["name"] = f"{new_hook.get('name')}-{image}" + new_hook["language"] = "docker_image" + new_hook[ + "entry" + ] = f'--entrypoint {new_hook.get("entry")} {get_environment_flag()} {dev_image}' ret_hooks = [] counter = 0 @@ -284,7 +285,10 @@ def get_new_hooks( if self._set_files_on_hook(hook, files): ret_hooks.append(hook) for hook in ret_hooks: - self._set_properties(hook, to_delete=["docker_image", "config_file_arg"]) + if hook.get("docker_image"): + hook.pop("docker_image") + if hook.get("config_file_arg"): + hook.pop("config_file_arg") return ret_hooks def _get_config_file_arg(self) -> Optional[Tuple]: diff --git a/demisto_sdk/commands/pre_commit/hooks/hook.py b/demisto_sdk/commands/pre_commit/hooks/hook.py index c8118c710a..6999adfb63 100644 --- a/demisto_sdk/commands/pre_commit/hooks/hook.py +++ b/demisto_sdk/commands/pre_commit/hooks/hook.py @@ -1,19 +1,17 @@ import re -from abc import ABC, abstractmethod from copy import deepcopy from pathlib import Path -from typing import Any, Dict, List, Optional, Set +from typing import Any, Dict, List, Set -from demisto_sdk.commands.common.constants import PreCommitModes from demisto_sdk.commands.common.logger import logger -class Hook(ABC): +class Hook: def __init__( self, hook: dict, repo: dict, - mode: Optional[PreCommitModes] = None, + mode: str = "", all_files: bool = False, input_mode: bool = False, ) -> None: @@ -24,15 +22,15 @@ def __init__( self.mode = mode self.all_files = all_files self.input_mode = input_mode + self._set_properties() - @abstractmethod def prepare_hook(self, **kwargs): """ This method should be implemented in each hook. Since we removed the base hook from the hooks list, we must add it back. So "self.hooks.append(self.base_hook)" or copy of the "self.base_hook" should be added anyway. """ - ... + self.hooks.append(deepcopy(self.base_hook)) def _set_files_on_hook(self, hook: dict, files) -> int: """ @@ -85,37 +83,30 @@ def _get_property(self, name, default=None): Args: name: the key to get from the config default: the default value to return - Returns: The value from the base hook """ ret = None if self.mode: - ret = self.base_hook.get(f"{name}:{self.mode.value}") + ret = self.base_hook.get(f"{name}:{self.mode}") return ret or self.base_hook.get(name, default) - def _set_properties(self, hook, to_delete=()): + def _set_properties(self): """ - Will alter the new hook, setting the properties that don't need unique behavior - For any propery x, if x isn't already defined, x will be set according to the mode provided. - + For any property x, if x isn't already defined, x will be set according to the mode provided. For example, given an input - args: 123 args:nightly 456 - - if the mode provided is nightly, args will be set to 456. Otherwise, the default (key with no :) will be taken - - Args: - hook: the hook to modify - to_delete: keys on the demisto config that we dont want to pass to precommit - + if the mode provided is nightly, args will be set to 456. Otherwise, the default (key with no :) will be taken. + Update the base_hook accordingly. """ + hook: Dict = {} for full_key in self.base_hook: key = full_key.split(":")[0] - if hook.get(key) or key in to_delete: + if hook.get(key): continue if prop := self._get_property(key): hook[key] = prop + self.base_hook = hook def join_files(files: Set[Path], separator: str = "|") -> str: diff --git a/demisto_sdk/commands/pre_commit/hooks/ruff.py b/demisto_sdk/commands/pre_commit/hooks/ruff.py index d0d3c4b09e..18d6491c39 100644 --- a/demisto_sdk/commands/pre_commit/hooks/ruff.py +++ b/demisto_sdk/commands/pre_commit/hooks/ruff.py @@ -2,8 +2,11 @@ from pathlib import Path from typing import Any, Dict, Set -from demisto_sdk.commands.common.constants import PreCommitModes -from demisto_sdk.commands.pre_commit.hooks.hook import Hook, join_files +from demisto_sdk.commands.pre_commit.hooks.hook import ( + Hook, + join_files, + safe_update_hook_args, +) class RuffHook(Hook): @@ -36,13 +39,10 @@ def prepare_hook( "name": f"ruff-py{python_version}", } hook.update(deepcopy(self.base_hook)) - hook["args"] = [ - f"--target-version={self._python_version_to_ruff(python_version)}", - ] - if self.mode == PreCommitModes.NIGHTLY: - hook["args"].append("--config=nightly_ruff.toml") - else: - hook["args"].append("--fix") + target_version = ( + f"--target-version={self._python_version_to_ruff(python_version)}" + ) + safe_update_hook_args(hook, target_version) if github_actions: hook["args"].append("--format=github") hook["files"] = join_files(python_version_to_files[python_version]) diff --git a/demisto_sdk/commands/pre_commit/hooks/sourcery.py b/demisto_sdk/commands/pre_commit/hooks/sourcery.py index edd16cede5..aba95e67dd 100644 --- a/demisto_sdk/commands/pre_commit/hooks/sourcery.py +++ b/demisto_sdk/commands/pre_commit/hooks/sourcery.py @@ -4,7 +4,6 @@ from typing import Any, Dict from demisto_sdk.commands.common import tools -from demisto_sdk.commands.common.constants import PreCommitModes from demisto_sdk.commands.pre_commit.hooks.hook import Hook, join_files @@ -44,8 +43,6 @@ def prepare_hook( hook["args"].append( f"--config={self._get_temp_config_file(config_file_path, python_version)}" ) - if not self.mode == PreCommitModes.NIGHTLY: - hook["args"].append("--fix") hook["files"] = join_files(python_version_to_files[python_version]) self.hooks.append(hook) diff --git a/demisto_sdk/commands/pre_commit/hooks/validate_format.py b/demisto_sdk/commands/pre_commit/hooks/validate_format.py index 759c92c8a7..ad84795acf 100644 --- a/demisto_sdk/commands/pre_commit/hooks/validate_format.py +++ b/demisto_sdk/commands/pre_commit/hooks/validate_format.py @@ -1,8 +1,11 @@ from pathlib import Path from typing import Iterable, Optional -from demisto_sdk.commands.common.constants import PreCommitModes -from demisto_sdk.commands.pre_commit.hooks.hook import Hook, join_files +from demisto_sdk.commands.pre_commit.hooks.hook import ( + Hook, + join_files, + safe_update_hook_args, +) class ValidateFormatHook(Hook): @@ -13,14 +16,14 @@ def prepare_hook(self, files_to_run: Optional[Iterable[Path]], **kwargs): In case of an input or all files without nightly, runs validate/format on the given files. Otherwise runs validate/format with the -g flag. Args: - input_files (Optional[Iterable[Path]]): The input files to validate. Defaults to None. + files_to_run (Optional[Iterable[Path]]): The input files to validate. Defaults to None. """ - if self.mode == PreCommitModes.NIGHTLY and self.all_files: - self.base_hook["args"].append("-a") + if self.mode and self.mode.lower() == "nightly" and self.all_files: + safe_update_hook_args(self.base_hook, "-a") elif self.input_mode or self.all_files: - self.base_hook["args"].append("-i") + safe_update_hook_args(self.base_hook, "-i") self.base_hook["args"].append(join_files(files_to_run, ",")) else: - self.base_hook["args"].append("-g") + safe_update_hook_args(self.base_hook, "-g") self.hooks.insert(self.hook_index, self.base_hook) diff --git a/demisto_sdk/commands/pre_commit/pre_commit_command.py b/demisto_sdk/commands/pre_commit/pre_commit_command.py index 1e67930f6a..509aaadde4 100644 --- a/demisto_sdk/commands/pre_commit/pre_commit_command.py +++ b/demisto_sdk/commands/pre_commit/pre_commit_command.py @@ -16,7 +16,6 @@ DEFAULT_PYTHON_VERSION, INTEGRATIONS_DIR, SCRIPTS_DIR, - PreCommitModes, ) from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH, PYTHONPATH from demisto_sdk.commands.common.git_util import GitUtil @@ -33,7 +32,7 @@ IntegrationScript, ) from demisto_sdk.commands.pre_commit.hooks.docker import DockerHook -from demisto_sdk.commands.pre_commit.hooks.hook import join_files +from demisto_sdk.commands.pre_commit.hooks.hook import Hook, join_files from demisto_sdk.commands.pre_commit.hooks.mypy import MypyHook from demisto_sdk.commands.pre_commit.hooks.pycln import PyclnHook from demisto_sdk.commands.pre_commit.hooks.ruff import RuffHook @@ -68,7 +67,7 @@ class PreCommitRunner: input_mode: bool all_files: bool - mode: Optional[PreCommitModes] + mode: str python_version_to_files: Dict[str, Set[Path]] demisto_sdk_commit_hash: str @@ -152,18 +151,28 @@ def prepare_hooks( "all_files": self.all_files, "input_mode": self.input_mode, } - PyclnHook(**hooks["pycln"], **kwargs).prepare_hook(PYTHONPATH) - RuffHook(**hooks["ruff"], **kwargs).prepare_hook( - self.python_version_to_files, IS_GITHUB_ACTIONS - ) - MypyHook(**hooks["mypy"], **kwargs).prepare_hook(self.python_version_to_files) - SourceryHook(**hooks["sourcery"], **kwargs).prepare_hook( - self.python_version_to_files, config_file_path=SOURCERY_CONFIG_PATH - ) - ValidateFormatHook(**hooks["validate"], **kwargs).prepare_hook( - self.files_to_run - ) - ValidateFormatHook(**hooks["format"], **kwargs).prepare_hook(self.files_to_run) + if "pycln" in hooks: + PyclnHook(**hooks.pop("pycln"), **kwargs).prepare_hook(PYTHONPATH) + if "ruff" in hooks: + RuffHook(**hooks.pop("ruff"), **kwargs).prepare_hook( + self.python_version_to_files, IS_GITHUB_ACTIONS + ) + if "mypy" in hooks: + MypyHook(**hooks.pop("mypy"), **kwargs).prepare_hook( + self.python_version_to_files + ) + if "sourcery" in hooks: + SourceryHook(**hooks.pop("sourcery"), **kwargs).prepare_hook( + self.python_version_to_files, config_file_path=SOURCERY_CONFIG_PATH + ) + if "validate" in hooks: + ValidateFormatHook(**hooks.pop("validate"), **kwargs).prepare_hook( + self.files_to_run + ) + if "format" in hooks: + ValidateFormatHook(**hooks.pop("format"), **kwargs).prepare_hook( + self.files_to_run + ) [ DockerHook(**hook, **kwargs).prepare_hook( files_to_run=self.files_to_run, run_docker_hooks=run_docker_hooks @@ -171,6 +180,11 @@ def prepare_hooks( for hook_id, hook in hooks.items() if hook_id.endswith("in-docker") ] + hooks_without_docker = [ + hook for hook_id, hook in hooks.items() if not hook_id.endswith("in-docker") + ] + for hook in hooks_without_docker: + Hook(**hook, **kwargs).prepare_hook() def run( self, @@ -191,6 +205,8 @@ def run( skipped_hooks.update(set(skip_hooks or ())) if not unit_test: skipped_hooks.add("run-unit-tests") + skipped_hooks.add("coverage-analyze") + skipped_hooks.add("merge-coverage-report") if validate and "validate" in skipped_hooks: skipped_hooks.remove("validate") if format and "format" in skipped_hooks: @@ -347,7 +363,7 @@ def pre_commit_manager( commited_only: bool = False, git_diff: bool = False, all_files: bool = False, - mode: Optional[PreCommitModes] = None, + mode: str = "", unit_test: bool = False, skip_hooks: Optional[List[str]] = None, validate: bool = False, @@ -367,7 +383,7 @@ def pre_commit_manager( commited_only (bool, optional): Whether to run on commited files only. Defaults to False. git_diff (bool, optional): Whether use git to determine precommit files. Defaults to False. all_files (bool, optional): Whether to run on all_files. Defaults to False. - mode (PreCommitModes, optional): The mode to run pre-commit in. Defaults to None. + mode (str): The mode to run pre-commit in. Defaults to empty str. test (bool, optional): Whether to run unit-tests. Defaults to False. skip_hooks (Optional[List[str]], optional): List of hooks to skip. Defaults to None. force_run_hooks (Optional[List[str]], optional): List for hooks to force run. Defaults to None. diff --git a/demisto_sdk/commands/pre_commit/tests/generic_docker_test.py b/demisto_sdk/commands/pre_commit/tests/generic_docker_test.py index 18260f5283..108f231136 100644 --- a/demisto_sdk/commands/pre_commit/tests/generic_docker_test.py +++ b/demisto_sdk/commands/pre_commit/tests/generic_docker_test.py @@ -2,7 +2,6 @@ import pytest -from demisto_sdk.commands.common.constants import PreCommitModes from demisto_sdk.commands.common.native_image import NativeImageConfig from demisto_sdk.commands.pre_commit.hooks.docker import ( DockerHook, @@ -41,7 +40,7 @@ def test_no_files(repo): @pytest.mark.parametrize( "mode, expected_text", [ - (PreCommitModes.NIGHTLY, ["i am the nightly args"]), + ("nightly", ["i am the nightly args"]), (None, ["i am some argument"]), ], ) @@ -108,7 +107,7 @@ def assert_get_prop_successful(mode, prop, expected_value): == expected_value ) - assert_get_prop_successful(PreCommitModes.NIGHTLY, "prop1", nightly_val) + assert_get_prop_successful("nightly", "prop1", nightly_val) assert_get_prop_successful(None, "prop1", value1) @@ -188,32 +187,27 @@ def test__set_properties(): When: Calling on same config with different modes Then: - The proper config is returned + The proper config is in base_hook. """ nightly_val = "nightlyval" value1 = "value1" - def assert_get_prop_successful(mode, expected_value, remove_props): - res = {} + def assert_get_prop_successful(mode, expected_value): hook = create_hook( { "prop1": value1, "prop1:nightly": nightly_val, "prop1:othermode": "someval", "other_prop": "whatever", - "remove_me": True, "nonused:mode": "isignored", } ) - DockerHook(**hook, mode=mode)._set_properties(res, to_delete=remove_props) - assert res == expected_value + docker_hook = DockerHook(**hook, mode=mode) + assert docker_hook.base_hook == expected_value assert_get_prop_successful( - PreCommitModes.NIGHTLY, + "nightly", {"prop1": nightly_val, "other_prop": "whatever"}, - ["remove_me"], - ) - assert_get_prop_successful( - None, {"prop1": value1, "other_prop": "whatever"}, ["remove_me"] ) + assert_get_prop_successful(None, {"prop1": value1, "other_prop": "whatever"}) diff --git a/demisto_sdk/commands/pre_commit/tests/pre_commit_test.py b/demisto_sdk/commands/pre_commit/tests/pre_commit_test.py index 2666205406..70de6474f3 100644 --- a/demisto_sdk/commands/pre_commit/tests/pre_commit_test.py +++ b/demisto_sdk/commands/pre_commit/tests/pre_commit_test.py @@ -4,11 +4,10 @@ import pytest import demisto_sdk.commands.pre_commit.pre_commit_command as pre_commit_command -from demisto_sdk.commands.common.constants import PreCommitModes from demisto_sdk.commands.common.handlers import DEFAULT_YAML_HANDLER as yaml from demisto_sdk.commands.common.legacy_git_tools import git_path from demisto_sdk.commands.pre_commit.hooks.docker import DockerHook -from demisto_sdk.commands.pre_commit.hooks.hook import join_files +from demisto_sdk.commands.pre_commit.hooks.hook import Hook, join_files from demisto_sdk.commands.pre_commit.hooks.mypy import MypyHook from demisto_sdk.commands.pre_commit.hooks.ruff import RuffHook from demisto_sdk.commands.pre_commit.hooks.validate_format import ValidateFormatHook @@ -130,6 +129,8 @@ def test_config_files(mocker, repo: Repo, is_test: bool): tests_we_should_skip = {"format", "validate", "secrets", "should_be_skipped"} if not is_test: tests_we_should_skip.add("run-unit-tests") + tests_we_should_skip.add("coverage-analyze") + tests_we_should_skip.add("merge-coverage-report") for m in mock_subprocess.call_args_list: assert set(m.kwargs["env"]["SKIP"].split(",")) == tests_we_should_skip @@ -164,17 +165,18 @@ def test_ruff_hook(github_actions): """ Testing ruff hook created successfully (the python version is correct and github action created successfully) """ - ruff_hook = create_hook({}) + ruff_hook = create_hook( + {"args": ["--fix"], "args:nightly": ["--config=nightly_ruff.toml"]} + ) RuffHook(**ruff_hook).prepare_hook(PYTHON_VERSION_TO_FILES, github_actions) python_version_to_ruff = {"3.8": "py38", "3.9": "py39", "3.10": "py310"} for (hook, python_version) in itertools.zip_longest( ruff_hook["repo"]["hooks"], PYTHON_VERSION_TO_FILES.keys() ): assert ( - hook["args"][0] - == f"--target-version={python_version_to_ruff[python_version]}" + f"--target-version={python_version_to_ruff[python_version]}" in hook["args"] ) - assert hook["args"][1] == "--fix" + assert "--fix" in hook["args"] assert hook["name"] == f"ruff-py{python_version}" assert hook["files"] == join_files(PYTHON_VERSION_TO_FILES[python_version]) if github_actions: @@ -185,10 +187,10 @@ def test_ruff_hook_nightly_mode(): """ Testing ruff hook created successfully in nightly mode (the --fix flag is not exist and the --config arg is added) """ - ruff_hook = create_hook({}) - RuffHook(**ruff_hook, mode=PreCommitModes.NIGHTLY).prepare_hook( - PYTHON_VERSION_TO_FILES + ruff_hook = create_hook( + {"args": ["--fix"], "args:nightly": ["--config=nightly_ruff.toml"]} ) + RuffHook(**ruff_hook, mode="nightly").prepare_hook(PYTHON_VERSION_TO_FILES) for (hook, _) in itertools.zip_longest( ruff_hook["repo"]["hooks"], PYTHON_VERSION_TO_FILES.keys() @@ -203,7 +205,7 @@ def test_validate_format_hook_nightly_mode_and_all_files(): Testing validate_format hook created successfully (the -a flag is added and the -i arg is not exist) """ validate_format_hook = create_hook({"args": []}) - kwargs = {"mode": PreCommitModes.NIGHTLY, "all_files": True} + kwargs = {"mode": "nightly", "all_files": True} ValidateFormatHook(**validate_format_hook, **kwargs).prepare_hook( PYTHON_VERSION_TO_FILES ) @@ -218,7 +220,7 @@ def test_validate_format_hook_nightly_mode(): Testing validate_format hook created successfully (the -i arg is added and the -a flag is not exist, even in nightly mode) """ validate_format_hook = create_hook({"args": []}) - kwargs = {"mode": PreCommitModes.NIGHTLY, "input_mode": True} + kwargs = {"mode": "nightly", "input_mode": True} ValidateFormatHook(**validate_format_hook, **kwargs).prepare_hook( PYTHON_VERSION_TO_FILES ) @@ -369,6 +371,48 @@ def test_exclude_python2_of_non_supported_hooks(mocker, repo: Repo): assert "file1.py" in hook["hook"]["exclude"] +args = [ + "-i", + ".coverage", + "--report-dir", + "coverage_report", + "--report-type", + "all", + "--previous-coverage-report-url", + "https://storage.googleapis.com/marketplace-dist-dev/code-coverage-reports/coverage-min.json", +] +args_nightly = [ + "-i", + ".coverage", + "--report-dir", + "coverage_report", + "--report-type", + "all", + "--allowed-coverage-degradation-percentage", + "100", +] + + +@pytest.mark.parametrize( + "mode, expected_args", [(None, args), ("nightly", args_nightly)] +) +def test_coverage_analyze_general_hook(mode, expected_args): + """ + Given: + - A hook and kwargs. + When: + - pre-commit command is running. + Then: + - Make sure that the coverage-analyze hook was created successfully. + """ + + coverage_analyze_hook = create_hook({"args": args, "args:nightly": args_nightly}) + kwargs = {"mode": mode, "all_files": False, "input_mode": True} + Hook(**coverage_analyze_hook, **kwargs).prepare_hook() + hook_args = coverage_analyze_hook["repo"]["hooks"][0]["args"] + assert expected_args == hook_args + + @pytest.mark.parametrize( "hook, expected_result", [ diff --git a/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py b/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py index cc2cab48be..1cd1bf737b 100644 --- a/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py +++ b/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py @@ -1,12 +1,8 @@ import os -import shutil -import sqlite3 -import tempfile import traceback from pathlib import Path from typing import List -import coverage from junitparser import JUnitXml import demisto_sdk.commands.common.docker_helper as docker_helper @@ -17,7 +13,6 @@ from demisto_sdk.commands.content_graph.objects.integration_script import ( IntegrationScript, ) -from demisto_sdk.commands.coverage_analyze.helpers import coverage_files from demisto_sdk.commands.lint.helpers import stream_docker_container_output DOCKER_PYTHONPATH = [ @@ -36,72 +31,6 @@ NO_TESTS_COLLECTED = 5 -def fix_coverage_report_path(coverage_file: Path) -> bool: - """ - - Args: - coverage_file: The coverage file to to fix (absolute file). - - Returns: - True if the file was fixed, False otherwise. - - Notes: - the .coverage files contain all the files list with their absolute path. - but our tests (pytest step) are running inside a docker container. - so we have to change the path to the correct one. - - """ - try: - logger.debug(f"Editing coverage report for {coverage_file}") - with tempfile.NamedTemporaryFile() as temp_file: - # we use a tempfile because the original file could be readonly, this way we assure we can edit it. - shutil.copy(coverage_file, temp_file.name) - with sqlite3.connect(temp_file.name) as sql_connection: - cursor = sql_connection.cursor() - files = cursor.execute("SELECT * FROM file").fetchall() - for id_, file in files: - if not file.startswith("/content"): - # means that the .coverage file is already fixed - continue - file = Path(file).relative_to("/content") - if ( - not (CONTENT_PATH / file).exists() - or file.parent.name - not in file.name # For example, in `QRadar_v3` directory we only care for `QRadar_v3.py` - ): - logger.debug(f"Removing {file} from coverage report") - cursor.execute( - "DELETE FROM file WHERE id = ?", (id_,) - ) # delete the file from the coverage report, as it is not relevant. - else: - cursor.execute( - "UPDATE file SET path = ? WHERE id = ?", - (str(CONTENT_PATH / file), id_), - ) - sql_connection.commit() - logger.debug("Done editing coverage report") - coverage_file.unlink() - shutil.copy(temp_file.name, coverage_file) - return True - except Exception: - logger.warning(f"Broken .coverage file found: {file}, deleting it") - file.unlink(missing_ok=True) - return False - - -def merge_coverage_report(): - coverage_path = CONTENT_PATH / ".coverage" - coverage_path.unlink(missing_ok=True) - cov = coverage.Coverage(data_file=coverage_path) - if not (files := coverage_files()): - logger.warning("No coverage files found, skipping coverage report.") - return - fixed_files = [file for file in files if fix_coverage_report_path(Path(file))] - cov.combine(fixed_files, keep=True) - cov.xml_report(outfile=str(CONTENT_PATH / "coverage.xml")) - logger.info(f"Coverage report saved to {CONTENT_PATH / 'coverage.xml'}") - - def unit_test_runner(file_paths: List[Path], verbose: bool = False) -> int: docker_client = docker_helper.init_global_docker_client() docker_base = docker_helper.get_docker() @@ -218,8 +147,4 @@ def unit_test_runner(file_paths: List[Path], verbose: bool = False) -> int: ) traceback.print_exc() exit_code = 1 - try: - merge_coverage_report() - except Exception as e: - logger.warning(f"Failed to merge coverage report: {e}") return exit_code diff --git a/demisto_sdk/scripts/merge_coverage_report.py b/demisto_sdk/scripts/merge_coverage_report.py new file mode 100644 index 0000000000..a32ada94de --- /dev/null +++ b/demisto_sdk/scripts/merge_coverage_report.py @@ -0,0 +1,87 @@ +import shutil +import sqlite3 +import tempfile +from pathlib import Path + +import coverage + +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH +from demisto_sdk.commands.common.logger import logger +from demisto_sdk.commands.coverage_analyze.helpers import coverage_files + + +def fix_coverage_report_path(coverage_file: Path) -> bool: + """ + Args: + coverage_file: The coverage file to to fix (absolute file). + Returns: + True if the file was fixed, False otherwise. + Notes: + the .coverage files contain all the files list with their absolute path. + but our tests (pytest step) are running inside a docker container. + so we have to change the path to the correct one. + """ + try: + logger.debug(f"Editing coverage report for {coverage_file}") + with tempfile.NamedTemporaryFile() as temp_file: + # we use a tempfile because the original file could be readonly, this way we assure we can edit it. + shutil.copy(coverage_file, temp_file.name) + with sqlite3.connect(temp_file.name) as sql_connection: + cursor = sql_connection.cursor() + files = cursor.execute("SELECT * FROM file").fetchall() + for id_, file in files: + if "conftest" in file: + cursor.execute( + "DELETE FROM file WHERE id = ?", (id_,) + ) # delete the file from the coverage report, as it is not relevant. + if not file.startswith("/content"): + # means that the .coverage file is already fixed + continue + file = Path(file).relative_to("/content") + if ( + not (CONTENT_PATH / file).exists() + or file.parent.name + != file.stem # For example, in `QRadar_v3` directory we only care for `QRadar_v3.py` + ): + logger.debug(f"Removing {file} from coverage report") + cursor.execute( + "DELETE FROM file WHERE id = ?", (id_,) + ) # delete the file from the coverage report, as it is not relevant. + else: + cursor.execute( + "UPDATE file SET path = ? WHERE id = ?", + (str(CONTENT_PATH / file), id_), + ) + sql_connection.commit() + logger.debug("Done editing coverage report") + coverage_file.unlink() + shutil.copy(temp_file.name, coverage_file) + return True + except Exception: + logger.warning(f"Broken .coverage file found: {file}, deleting it") + file.unlink(missing_ok=True) + return False + + +def merge_coverage_report(): + coverage_path = CONTENT_PATH / ".coverage" + coverage_path.unlink(missing_ok=True) + cov = coverage.Coverage(data_file=coverage_path) + if not (files := coverage_files()): + logger.warning("No coverage files found, skipping coverage report.") + return + fixed_files = [file for file in files if fix_coverage_report_path(Path(file))] + cov.combine(fixed_files) + cov.xml_report(outfile=str(CONTENT_PATH / "coverage.xml")) + logger.info(f"Coverage report saved to {CONTENT_PATH / 'coverage.xml'}") + + +def main(): + try: + merge_coverage_report() + except Exception as e: + logger.warning(f"Failed to merge coverage report: {e}") + + +if __name__ == "__main__": + SystemExit(main()) diff --git a/pyproject.toml b/pyproject.toml index 34e696f4e4..f3ce7ec4ea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,6 +19,7 @@ exclude = ["TestSuite/**", "*/tests/*", "tests"] [tool.poetry.scripts] demisto-sdk = "demisto_sdk.__main__:main" update-additional-dependencies = "demisto_sdk.scripts.update_additional_dependencies:main" +merge-coverage-report = "demisto_sdk.scripts.merge_coverage_report:main" [tool.poetry.dependencies] python = ">=3.8,<3.11"