diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a432f6f9f8..a050ebc557 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,6 +37,7 @@ repos: - cachetools==5.3.0 ; python_version >= "3.8" and python_version < "3.11" - certifi==2022.12.7 ; python_version >= "3.8" and python_version < "3.11" - cffi==1.15.1 ; python_version >= "3.8" and python_version < "3.11" + - cfgv==3.3.1 ; python_version >= "3.8" and python_version < "3.11" - chardet==5.1.0 ; python_version >= "3.8" and python_version < "3.11" - charset-normalizer==3.1.0 ; python_version >= "3.8" and python_version < "3.11" - click==8.1.3 ; python_version >= "3.8" and python_version < "3.11" @@ -51,9 +52,11 @@ repos: - demisto-py==3.2.10 ; python_version >= "3.8" and python_version < "3.11" - dictdiffer==0.9.0 ; python_version >= "3.8" and python_version < "3.11" - dictor==0.1.11 ; python_version >= "3.8" and python_version < "3.11" + - distlib==0.3.6 ; python_version >= "3.8" and python_version < "3.11" - docker==5.0.3 ; python_version >= "3.8" and python_version < "3.11" - docopt==0.6.2 ; python_version >= "3.8" and python_version < "3.11" - exceptiongroup==1.1.1 ; python_version >= "3.8" and python_version < "3.11" + - filelock==3.12.0 ; python_version >= "3.8" and python_version < "3.11" - flatten-dict==0.4.2 ; python_version >= "3.8" and python_version < "3.11" - freezegun==1.2.2 ; python_version >= "3.8" and python_version < "3.11" - future==0.18.3 ; python_version >= "3.8" and python_version < "3.11" @@ -74,6 +77,7 @@ repos: - grpcio-status==1.48.2 ; python_version >= "3.8" and python_version < "3.11" - grpcio==1.59.2 ; python_version >= "3.8" and python_version < "3.11" - humanfriendly==10.0 ; python_version >= "3.8" and python_version < "3.11" + - identify==2.5.24 ; python_version >= "3.8" and python_version < "3.11" - idna==3.4 ; python_version >= "3.8" and python_version < "3.11" - imagesize==1.4.1 ; python_version >= "3.8" and python_version < "3.11" - importlib-resources==5.12.0 ; python_version >= "3.8" and python_version < "3.11" @@ -93,6 +97,7 @@ repos: - neo4j==5.14.1 ; python_version >= "3.8" and python_version < "3.11" - networkx==2.8.8 ; python_version >= "3.8" and python_version < "3.11" - nltk==3.8.1 ; python_version >= "3.8" and python_version < "3.11" + - nodeenv==1.7.0 ; python_version >= "3.8" and python_version < "3.11" - ordered-set==4.1.0 ; python_version >= "3.8" and python_version < "3.11" - orjson==3.8.11 ; python_version >= "3.8" and python_version < "3.11" - packaging==23.1 ; python_version >= "3.8" and python_version < "3.11" @@ -102,6 +107,7 @@ repos: - pkgutil-resolve-name==1.3.10 ; python_version >= "3.8" and python_version < "3.9" - platformdirs==3.5.0 ; python_version >= "3.8" and python_version < "3.11" - pluggy==1.0.0 ; python_version >= "3.8" and python_version < "3.11" + - pre-commit==3.5.0 ; python_version >= "3.8" and python_version < "3.11" - prettytable==3.7.0 ; python_version >= "3.8" and python_version < "3.11" - proto-plus==1.22.3 ; python_version >= "3.8" and python_version < "3.11" - protobuf==3.19.6 ; python_version >= "3.8" and python_version < "3.11" @@ -176,6 +182,7 @@ repos: - tzlocal==4.3 ; python_version >= "3.8" and python_version < "3.11" - ujson==5.7.0 ; python_version >= "3.8" and python_version < "3.11" - urllib3==1.26.15 ; python_version >= "3.8" and python_version < "3.11" + - virtualenv==20.23.0 ; python_version >= "3.8" and python_version < "3.11" - vulture==2.7 ; python_version >= "3.8" and python_version < "3.11" - wcmatch==8.4.1 ; python_version >= "3.8" and python_version < "3.11" - wcwidth==0.2.6 ; python_version >= "3.8" and python_version < "3.11" diff --git a/CHANGELOG.md b/CHANGELOG.md index 832fb48cd5..44128a1c58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ * Added support for `skip` property in **pre-commit** hooks. * **generate-unit-tests** command will require installation with `pip install demisto-sdk[generate-unit-tests]`. * added the *IN150* and *IN161* errors to *allowed ignore errors* list. +* Added support for `env`, `copy_files` property in **pre-commit** docker hooks. +* Added support to run specific hooks in **pre-commit**. Use with `demisto-sdk pre-commit `. +* **Breaking change**: Removed the command **run-unit-tests**. Use `demisto-sdk pre-commit pytest-in-docker` instead. +* **Breaking change**: Removed the `--unit-test` argument in **pre-commit**. To skip unit tests, run with `--no-docker` or with `skip=pytest-in-docker`, ## 1.24.0 * Fixed an issue where the error was not clear when trying to retrieve the server version. diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py index 8f33253999..1df685a3b4 100644 --- a/demisto_sdk/__main__.py +++ b/demisto_sdk/__main__.py @@ -17,7 +17,7 @@ import logging import os from pathlib import Path -from typing import IO, Any, Dict, Iterable, Optional, Tuple, Union +from typing import IO, Any, Dict, List, Optional, Tuple, Union import typer from pkg_resources import DistributionNotFound, get_distribution @@ -3452,192 +3452,6 @@ def update_content_graph( ) -@main.command(short_help="Runs pre-commit hooks on the files in the repository") -@click.help_option("-h", "--help") -@click.option( - "-i", - "--input", - help="The path to the input file to run the command on.", - type=PathsParamType( - exists=True, resolve_path=True - ), # PathsParamType allows passing a list of paths -) -@click.option( - "-s", - "--staged-only", - help="Whether to run only on staged files", - is_flag=True, - default=False, -) -@click.option( - "--commited-only", - help="Whether to run on commited files only", - is_flag=True, - default=False, -) -@click.option( - "-g", - "--git-diff", - help="Whether to use git to determine which files to run on", - is_flag=True, - default=False, -) -@click.option( - "-a", - "--all-files", - help="Whether to run on all files", - is_flag=True, - default=False, -) -@click.option( - "--mode", - help="Special mode to run the pre-commit with", -) -@click.option( - "-ut/--no-ut", - "--unit-test/--no-unit-test", - help="Whether to run unit tests for content items", - default=False, -) -@click.option( - "--skip", - help="A comma separated list of precommit hooks to skip", -) -@click.option( - "--validate/--no-validate", - help="Whether to run demisto-sdk validate", - default=True, -) -@click.option( - "--format/--no-format", - help="Whether to run demisto-sdk format", - default=False, -) -@click.option( - "--secrets/--no-secrets", - help="Whether to run demisto-sdk secrets", - default=True, -) -@click.option( - "-v", - "--verbose", - help="Verbose output of pre-commit", - is_flag=True, - default=False, -) -@click.option( - "--show-diff-on-failure", - help="Show diff on failure", - is_flag=True, - default=False, -) -@click.option( - "--sdk-ref", - help="The demisto-sdk ref to use for the pre-commit hooks", - default="", -) -@click.option( - "--dry-run", - help="Whether to run the pre-commit hooks in dry-run mode, which will only create the config file", - is_flag=True, - default=False, -) -@click.argument( - "file_paths", - nargs=-1, - type=click.Path(exists=True, resolve_path=True, path_type=Path), -) -@click.option( - "--docker/--no-docker", - help="Whether to run docker based hooks or not.", - default=True, - is_flag=True, -) -@click.pass_context -@logging_setup_decorator -def pre_commit( - ctx, - input: str, - staged_only: bool, - commited_only: bool, - git_diff: bool, - all_files: bool, - mode: Optional[str], - unit_test: bool, - skip: str, - validate: bool, - format: bool, - secrets: bool, - verbose: bool, - show_diff_on_failure: bool, - sdk_ref: str, - file_paths: Iterable[Path], - dry_run: bool, - docker: bool, - **kwargs, -): - from demisto_sdk.commands.pre_commit.pre_commit_command import pre_commit_manager - - if file_paths and input: - logger.info( - "Both `--input` parameter and `file_paths` arguments were provided. Will use the `--input` parameter." - ) - input_files = [] - if input: - input_files = [Path(i) for i in input.split(",") if i] - elif file_paths: - input_files = list(file_paths) - if skip: - skip = skip.split(",") # type: ignore[assignment] - - sys.exit( - pre_commit_manager( - input_files, - staged_only, - commited_only, - git_diff, - all_files, - mode, - unit_test, - skip, - validate, - format, - secrets, - verbose, - show_diff_on_failure, - run_docker_hooks=docker, - sdk_ref=sdk_ref, - dry_run=dry_run, - ) - ) - - -@main.command(short_help="Run unit tests in a docker for integrations and scripts") -@click.help_option("-h", "--help") -@click.option( - "-i", - "--input", - type=PathsParamType( - exists=True, resolve_path=True - ), # PathsParamType allows passing a list of paths - help="The path of the content pack/file to validate specifically.", -) -@click.option( - "-v", "--verbose", is_flag=True, default=False, help="Verbose output of unit tests" -) -@click.argument("file_paths", nargs=-1, type=click.Path(exists=True, resolve_path=True)) -@click.pass_context -@logging_setup_decorator -def run_unit_tests( - ctx, input: str, file_paths: Tuple[str, ...], verbose: bool, **kwargs -): - if input: - file_paths = tuple(input.split(",")) - from demisto_sdk.commands.run_unit_tests.unit_tests_runner import unit_test_runner - - sys.exit(unit_test_runner(file_paths, verbose)) - - @main.command(short_help="Setup integration environments") @click.option( "-i", @@ -3708,11 +3522,106 @@ def exit_from_program(result=0, **kwargs): sys.exit(result) +# ====================== Pre-Commit ====================== # +pre_commit_app = typer.Typer(name="Pre-Commit") + + +@pre_commit_app.command() +def pre_commit( + input_files: Optional[List[Path]] = typer.Option( + None, + "-i", + "--input", + "--files", + exists=True, + dir_okay=True, + resolve_path=True, + show_default=False, + help=("The paths to run pre-commit on. May pass multiple paths."), + ), + staged_only: bool = typer.Option( + False, "--staged-only", help="Whether to run only on staged files" + ), + commited_only: bool = typer.Option( + False, "--commited-only", help="Whether to run on commited files only" + ), + git_diff: bool = typer.Option( + False, + "--git-diff", + "-g", + help="Whether to use git to determine which files to run on", + ), + all_files: bool = typer.Option( + False, "--all-files", "-a", help="Whether to run on all files" + ), + mode: str = typer.Option( + "", "--mode", help="Special mode to run the pre-commit with" + ), + skip: Optional[List[str]] = typer.Option( + None, "--skip", help="A list of precommit hooks to skip" + ), + validate: bool = typer.Option( + True, "--validate/--no-validate", help="Whether to run demisto-sdk validate" + ), + format: bool = typer.Option( + False, "--format/--no-format", help="Whether to run demisto-sdk format" + ), + secrets: bool = typer.Option( + True, "--secrets/--no-secrets", help="Whether to run demisto-sdk secrets" + ), + verbose: bool = typer.Option( + False, "-v", "--verbose", help="Verbose output of pre-commit" + ), + show_diff_on_failure: bool = typer.Option( + False, "--show-diff-on-failure", help="Show diff on failure" + ), + dry_run: bool = typer.Option( + False, + "--dry-run", + help="Whether to run the pre-commit hooks in dry-run mode, which will only create the config file", + ), + docker: bool = typer.Option( + True, "--docker/--no-docker", help="Whether to run docker based hooks or not." + ), + run_hook: Optional[str] = typer.Argument(None, help="A specific hook to run"), +): + from demisto_sdk.commands.pre_commit.pre_commit_command import pre_commit_manager + + return_code = pre_commit_manager( + input_files, + staged_only, + commited_only, + git_diff, + all_files, + mode, + skip, + validate, + format, + secrets, + verbose, + show_diff_on_failure, + run_docker_hooks=docker, + dry_run=dry_run, + run_hook=run_hook, + ) + if return_code: + raise typer.Exit(1) + + +main.add_command(typer.main.get_command(pre_commit_app), "pre-commit") + + # ====================== modeling-rules command group ====================== # -app = typer.Typer(name="modeling-rules", hidden=True, no_args_is_help=True) -app.command("test", no_args_is_help=True)(test_modeling_rule.test_modeling_rule) -app.command("init-test-data", no_args_is_help=True)(init_test_data.init_test_data) -typer_click_object = typer.main.get_command(app) +modeling_rules_app = typer.Typer( + name="modeling-rules", hidden=True, no_args_is_help=True +) +modeling_rules_app.command("test", no_args_is_help=True)( + test_modeling_rule.test_modeling_rule +) +modeling_rules_app.command("init-test-data", no_args_is_help=True)( + init_test_data.init_test_data +) +typer_click_object = typer.main.get_command(modeling_rules_app) main.add_command(typer_click_object, "modeling-rules") app_generate_modeling_rules = typer.Typer( diff --git a/demisto_sdk/commands/common/content_constant_paths.py b/demisto_sdk/commands/common/content_constant_paths.py index 7de849f110..973c59a0d4 100644 --- a/demisto_sdk/commands/common/content_constant_paths.py +++ b/demisto_sdk/commands/common/content_constant_paths.py @@ -22,13 +22,19 @@ PYTHONPATH = [ - Path(CONTENT_PATH), - Path(CONTENT_PATH / "Packs" / "Base" / "Scripts" / "CommonServerPython"), - Path(CONTENT_PATH / TESTS_DIR / "demistomock"), + Path(CONTENT_PATH).absolute(), + Path(CONTENT_PATH / "Packs" / "Base" / "Scripts" / "CommonServerPython").absolute(), + Path(CONTENT_PATH / TESTS_DIR / "demistomock").absolute(), Path(__file__).parent.parent / "lint" / "resources" / "pylint_plugins", ] try: - PYTHONPATH.extend(Path(CONTENT_PATH / "Packs" / "ApiModules" / "Scripts").iterdir()) + PYTHONPATH.extend( + ( + path.absolute() + for path in Path( + CONTENT_PATH / "Packs" / "ApiModules" / "Scripts" + ).iterdir() + ) + ) except FileNotFoundError: logger.info("ApiModules not found, skipping adding to PYTHONPATH") - pass diff --git a/demisto_sdk/commands/common/docker_helper.py b/demisto_sdk/commands/common/docker_helper.py index 6e4516cf29..9f5088c114 100644 --- a/demisto_sdk/commands/common/docker_helper.py +++ b/demisto_sdk/commands/common/docker_helper.py @@ -169,15 +169,40 @@ def pull_image(image: str) -> docker.models.images.Image: Get a local docker image, or pull it when unavailable. """ docker_client = init_global_docker_client(log_prompt="pull_image") - try: return docker_client.images.get(image) + except docker.errors.ImageNotFound: logger.debug(f"docker {image=} not found locally, pulling") ret = docker_client.images.pull(image) logger.debug(f"pulled docker {image=} successfully") return ret + @staticmethod + def is_image_available( + image: str, + ) -> bool: + docker_client = init_global_docker_client(log_prompt="get_image") + try: + docker_client.images.get(image) + return True + except docker.errors.ImageNotFound as e: + if ":" not in image: + repo = image + tag = "latest" + elif image.count(":") > 1: + raise ValueError(f"Invalid docker image: {image}") from e + else: + try: + repo, tag = image.split(":") + token = _get_docker_hub_token(repo) + if _get_image_digest(repo, tag, token): + return True + except RuntimeError as e: + logger.debug(f"Error getting image data {image}: {e}") + return False + return False + @staticmethod def copy_files_container( container: docker.models.containers.Container, files: FILES_SRC_TARGET @@ -302,13 +327,20 @@ def create_image( self.push_image(image, log_prompt=log_prompt) return image - def pull_or_create_test_image( + @staticmethod + def get_image_registry(image: str) -> str: + if os.getenv("CONTENT_GITLAB_CI") and "code.pan.run" not in image: + return f"docker-io.art.code.pan.run/{image}" + return image + + def get_or_create_test_image( self, base_image: str, container_type: str = TYPE_PYTHON, python_version: Optional[int] = None, additional_requirements: Optional[List[str]] = None, push: bool = False, + should_pull: bool = True, log_prompt: str = "", ) -> Tuple[str, str]: """This will generate the test image for the given base image. @@ -320,6 +352,7 @@ def pull_or_create_test_image( Returns: The test image name and errors to create it if any """ + errors = "" if ( not python_version @@ -344,9 +377,14 @@ def pull_or_create_test_image( identifier = hashlib.md5( "\n".join(sorted(pip_requirements)).encode("utf-8") ).hexdigest() + test_docker_image = ( f'{base_image.replace("demisto", "devtestdemisto")}-{identifier}' ) + if not should_pull and self.is_image_available(test_docker_image): + return test_docker_image, errors + base_image = self.get_image_registry(base_image) + test_docker_image = self.get_image_registry(test_docker_image) try: logger.debug( diff --git a/demisto_sdk/commands/content_graph/objects/integration_script.py b/demisto_sdk/commands/content_graph/objects/integration_script.py index 9ad9102b8e..a160e83f76 100644 --- a/demisto_sdk/commands/content_graph/objects/integration_script.py +++ b/demisto_sdk/commands/content_graph/objects/integration_script.py @@ -37,7 +37,9 @@ def python_version(self) -> Optional[str]: """ Get the python version from the script/integration docker-image in case it's a python image """ - if python_version := get_python_version(self.docker_image): + if self.type == "python" and ( + python_version := get_python_version(self.docker_image) + ): return str(python_version) return None diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index dee7e3713d..7cf96d6970 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -929,7 +929,7 @@ def _docker_image_create(self, docker_base_image: List[Any]) -> Tuple[str, str]: py_ver = None if docker_base_image[1] != -1: py_ver = parse(docker_base_image[1]).major # type: ignore - test_image_name, errors = docker_base.pull_or_create_test_image( + test_image_name, errors = docker_base.get_or_create_test_image( docker_base_image[0], additional_requirements=self._facts["additional_requirements"], container_type=self._pkg_lint_status["pack_type"], diff --git a/demisto_sdk/commands/pre_commit/README.md b/demisto_sdk/commands/pre_commit/README.md index 142f6f76ab..77701a4120 100644 --- a/demisto_sdk/commands/pre_commit/README.md +++ b/demisto_sdk/commands/pre_commit/README.md @@ -33,6 +33,27 @@ You can set this as follows. 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. +## Hooks +Hooks can be set in the `.pre-commit-config_template.yaml` file. The syntax is similar to the official [`pre-commit` hooks](https://pre-commit.com/#new-hooks). But the command allows more keys to be set: + +### Skip key +In order to skip certain hook from running, you can add a `skip` key to the hook configuration. +```yaml +- id: sample-hook + skip: true +``` +This key could be use together with [mode](#modes), to skip certain hook when running in a specific mode. + +### Needs key +Needs keys allow to define dependencies between hooks. If a hook with `needs` is skipped, hooks that depend on it will also be skipped. +In this example, both hooks will be skipped. +```yaml +- id: sample-hook + skip: true +- id: needs-example + needs: ["sample-hook"] +``` + ## Steps ### External tools @@ -59,7 +80,6 @@ The following SDK commands are automatically run - [validate](https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/validate/README.md) - [format](https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/format/README.md) - [secrets](https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/secrets/README.md) -- run-unit-tests: Runs the unit tests in an environment matching the content. ### Docker hooks To run a command in a script's relevant container you can set up a Docker Hook. @@ -77,6 +97,22 @@ 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). +#### The env key +Some commands require environment variables to be set. To configure environment variables for docker hooks you can set the `env` key. Here is an example with pytest. +```yaml + - id: simple-in-docker + env: + DEMISTO_SDK__PYTEST__TESTS_PATH: Tests/scripts +``` + +#### The copy_files key +Some hooks require several files to exist in the same directory as the code file in order to run properly. To configure this you can set the `copy_files` key as follows: +```yaml +- id: simple-in-docker + copy_files: + - Tests/scripts/conftest.py +``` + #### 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 b781f8ebab..519fe67f4f 100644 --- a/demisto_sdk/commands/pre_commit/hooks/docker.py +++ b/demisto_sdk/commands/pre_commit/hooks/docker.py @@ -1,7 +1,11 @@ import functools +import os +import shutil +import subprocess import time from collections import defaultdict from copy import deepcopy +from functools import lru_cache from pathlib import Path from typing import Dict, Iterable, List, Optional, Set, Tuple @@ -25,17 +29,17 @@ from demisto_sdk.commands.lint.linter import DockerImageFlagOption from demisto_sdk.commands.pre_commit.hooks.hook import Hook -NO_CONFIG_VALUE = None +NO_SPLIT = None -@functools.lru_cache +@lru_cache() def get_docker_python_path() -> str: """ precommit by default mounts the content repo to source. This means CommonServerPython's path is /src/Packs/Base/...CSP.py Returns: A PYTHONPATH formatted string """ - path_to_replace = str(Path(CONTENT_PATH)) + path_to_replace = str(Path(CONTENT_PATH).absolute()) docker_path = [str(path).replace(path_to_replace, "/src") for path in PYTHONPATH] path = ":".join(docker_path) logger.debug(f"pythonpath in docker being set to {path}") @@ -56,9 +60,7 @@ def with_native_tags( """ docker_flags = set(docker_image_flag.split(",")) - all_tags_to_files: Dict[str, List[Tuple[Path, IntegrationScript]]] = defaultdict( - list - ) + all_tags_to_files = defaultdict(list) native_image_config = NativeImageConfig.get_instance() for image, scripts in tags_to_files.items(): @@ -91,7 +93,7 @@ def docker_tag_to_runfiles( Returns: A dict of image to List of files(Tuple[path, obj]) including native images """ - tags_to_files: Dict[str, List[Tuple[Path, IntegrationScript]]] = defaultdict(list) + tags_to_files = defaultdict(list) for file, obj in files_to_run: if not obj: continue @@ -101,61 +103,80 @@ def docker_tag_to_runfiles( @functools.lru_cache(maxsize=512) -def devtest_image(image_tag, is_powershell) -> str: +def devtest_image( + image_tag: str, + is_powershell: bool, + should_pull: bool, +) -> str: """ We need to add test dependencies on the image. In the future we could add "additional_dependencies" as a template config arg and pass it through here Args: image_tag: the base image tag is_powershell: if the image is a powershell based image - + should_pull: if true, don't pull images on background Returns: The build and pulled dev image """ - all_errors: list = [] - for _ in range(2): # retry it once - logger.info(f"getting devimage for {image_tag}, {is_powershell=}") - image, errors = get_docker().pull_or_create_test_image( - base_image=image_tag, - container_type=TYPE_PWSH if is_powershell else TYPE_PYTHON, - push=docker_login(docker_client=init_global_docker_client()), - log_prompt="DockerHook", - ) - if not errors: - return image - all_errors.append(errors) - raise DockerException(all_errors) + docker_base = get_docker() + image, errors = docker_base.get_or_create_test_image( + base_image=image_tag, + container_type=TYPE_PWSH if is_powershell else TYPE_PYTHON, + push=docker_login(docker_client=init_global_docker_client()), + should_pull=False, + log_prompt="DockerHook", + ) + if not errors: + if not should_pull: + # pull images in background + subprocess.Popen( + ["docker", "pull", image], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + return image + raise DockerException(errors) -def get_environment_flag() -> str: +def get_environment_flag(env: dict) -> str: """ The env flag needed to run python scripts in docker """ - return f'--env "PYTHONPATH={get_docker_python_path()}"' - - -def _split_by_config_file(files, config_arg: Optional[Tuple]): + env_flag = f'--env "PYTHONPATH={get_docker_python_path()}"' + for key, value in env.items(): + env_flag += f' --env "{key}={value}"' + if os.getenv("GITHUB_ACTIONS"): + env_flag += " --env GITHUB_ACTIONS=true" + return env_flag + + +def _split_by_objects( + files_with_objects: List[Tuple[Path, IntegrationScript]], + config_arg: Optional[Tuple], + split_by_obj: bool = False, +) -> Dict[Optional[IntegrationScript], Set[Tuple[Path, IntegrationScript]]]: """ Will group files into groups that share the same configuration file. If there is no config file, they get set to the NO_CONFIG_VALUE group Args: files: the files to split config_arg: a tuple, argument_name, file_name + split_by_obj: a boolean. If true it will split all the objects into separate hooks. Returns: a dict where the keys are the names of the folder of the config and the value is a set of files for that config """ - if not config_arg: - return {NO_CONFIG_VALUE: files} - folder_to_files = defaultdict(set) + object_to_files: Dict[ + Optional[IntegrationScript], Set[Tuple[Path, IntegrationScript]] + ] = defaultdict(set) - for file in files: - if (file.parent / config_arg[1]).exists(): - folder_to_files[str(file.parent)].add(file) + for file, obj in files_with_objects: + if split_by_obj or (config_arg and (obj.path.parent / config_arg[1]).exists()): + object_to_files[obj].add((file, obj)) else: - folder_to_files[NO_CONFIG_VALUE].add(file) # type:ignore + object_to_files[NO_SPLIT].add((file, obj)) - return folder_to_files + return object_to_files class DockerHook(Hook): @@ -166,7 +187,7 @@ class DockerHook(Hook): def prepare_hook( self, files_to_run_with_objects: Iterable[Tuple[Path, Optional[IntegrationScript]]], - run_docker_hooks, + dry_run: bool, ): """ Group all the files by dockerimages @@ -174,12 +195,9 @@ def prepare_hook( Get the devimage for each image Args: files_to_run: all files to run on - run_docker_hooks: bool - Whether to run docker based hooks or skip them. - + dry_run: bool: Whether we are in dry run or not, affects pulling images. """ - if not run_docker_hooks: - logger.debug("Skipping docker preparation since run_docker_hooks is False") - return + start_time = time.time() filtered_files = self.filter_files_matching_hook_config( (file for file, _ in files_to_run_with_objects) @@ -202,27 +220,39 @@ def prepare_hook( logger.debug( f"Elapsed time to gather tags to files: {end_time - start_time} seconds" ) + if copy_files := self._get_property("copy_files"): + all_objects = {obj for _, obj in filtered_files_with_objects if obj} + for obj in all_objects: + for file in copy_files: + source: Path = CONTENT_PATH / file + target = obj.path.parent / Path(file).name + if source != target and source.exists(): + shutil.copy( + CONTENT_PATH / file, obj.path.parent / Path(file).name + ) + split_by_obj = self._get_property("split_by_object", False) config_arg = self._get_config_file_arg() - start_time = time.time() logger.info(f"{len(tag_to_files_objs)} images were collected from files") logger.debug(f'collected images: {" ".join(tag_to_files_objs.keys())}') for image, files_with_objects in sorted( tag_to_files_objs.items(), key=lambda item: item[0] ): - - paths = {file for file, obj in files_with_objects} - folder_to_files = _split_by_config_file(paths, config_arg) + object_to_files = _split_by_objects( + files_with_objects, config_arg, split_by_obj + ) image_is_powershell = any( obj.is_powershell for _, obj in files_with_objects ) - dev_image = devtest_image( # consider moving to before loop and threading. - image, image_is_powershell + dev_image = devtest_image(image, image_is_powershell, dry_run) + hooks = self.get_new_hooks( + dev_image, + image, + object_to_files, + config_arg, ) - hooks = self.get_new_hooks(dev_image, image, folder_to_files, config_arg) self.hooks.extend(hooks) - end_time = time.time() logger.info( f"DockerHook - Elapsed time to prep all the images: {end_time - start_time} seconds" @@ -232,7 +262,9 @@ def get_new_hooks( self, dev_image, image, - folder_to_files: Dict[str, Set[Path]], + object_to_files_with_objects: Dict[ + Optional[IntegrationScript], Set[Tuple[Path, IntegrationScript]] + ], config_arg: Optional[Tuple], ): """ @@ -240,7 +272,7 @@ def get_new_hooks( Args: dev_image: The actual image to run on image: name of the base image (for naming) - folder_to_files: A dict where the key is the folder and value is the set of files to run together. + object_to_files_with_objects: A dict where the key is the object (or None) and value is the set of files to run together. config_arg: The config arg to set where relevant. This will be appended to the end of "args" Returns: All the hooks to be appended for this image @@ -249,33 +281,48 @@ def get_new_hooks( new_hook["id"] = f"{new_hook.get('id')}-{image}" new_hook["name"] = f"{new_hook.get('name')}-{image}" new_hook["language"] = "docker_image" + env = new_hook.pop("env", {}) new_hook[ "entry" - ] = f'--entrypoint {new_hook.get("entry")} {get_environment_flag()} {dev_image}' - + ] = f'--entrypoint {new_hook.get("entry")} {get_environment_flag(env)} --quiet {dev_image}' ret_hooks = [] - counter = 0 - for folder, files in folder_to_files.items(): + for ( + integration_script, + files_with_objects, + ) in object_to_files_with_objects.items(): + files = {file for file, _ in files_with_objects} hook = deepcopy(new_hook) - if config_arg and folder is not NO_CONFIG_VALUE: - args = deepcopy(self._get_property("args", [])) - args.extend( - [ - config_arg[0], # type:ignore - str(list(files)[0].parent / config_arg[1]), # type:ignore - ] # type:ignore - ) # type:ignore - hook["args"] = args - hook["id"] = f"{hook['id']}-{counter}" # for uniqueness - hook["name"] = f"{hook['name']}-{counter}" - counter += 1 - if self._set_files_on_hook(hook, files): + if integration_script is not None: + if config_arg: + args = deepcopy(self._get_property("args", [])) + args.extend( + [ + config_arg[0], + str( + ( + integration_script.path.parent / config_arg[1] + ).relative_to(CONTENT_PATH) + ), + ] + ) + hook["args"] = args + hook[ + "id" + ] = f"{hook['id']}-{integration_script.object_id}" # for uniqueness + hook[ + "name" + ] = f"{hook['name']}-{integration_script.object_id}" # for uniqueness + if self._set_files_on_hook( + hook, files, should_filter=False + ): # no need to filter again, we have only filtered files + # disable multiprocessing on hook + hook["require_serial"] = True ret_hooks.append(hook) for hook in ret_hooks: - if hook.get("docker_image"): - hook.pop("docker_image") - if hook.get("config_file_arg"): - hook.pop("config_file_arg") + hook.pop("docker_image", None) + hook.pop("config_file_arg", None) + hook.pop("copy_files", None) + hook.pop("split_by_object", None) 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 36d5538100..bc5ee1c37b 100644 --- a/demisto_sdk/commands/pre_commit/hooks/hook.py +++ b/demisto_sdk/commands/pre_commit/hooks/hook.py @@ -1,3 +1,4 @@ +import os import re from copy import deepcopy from pathlib import Path @@ -5,6 +6,8 @@ from demisto_sdk.commands.common.logger import logger +PROPERTIES_TO_DELETE = {"needs"} + class Hook: def __init__( @@ -32,7 +35,9 @@ def prepare_hook(self, **kwargs): """ self.hooks.append(deepcopy(self.base_hook)) - def _set_files_on_hook(self, hook: dict, files) -> int: + def _set_files_on_hook( + self, hook: dict, files: Iterable[Path], should_filter: bool = True + ) -> int: """ Args: @@ -42,9 +47,10 @@ def _set_files_on_hook(self, hook: dict, files) -> int: Returns: the number of files that ultimately are set on the hook. Use this to decide if to run the hook at all """ - files_to_run_on_hook = self.filter_files_matching_hook_config(files) + files_to_run_on_hook = set(files) + if should_filter: + files_to_run_on_hook = self.filter_files_matching_hook_config(files) hook["files"] = join_files(files_to_run_on_hook) - return len(files_to_run_on_hook) def filter_files_matching_hook_config( @@ -116,12 +122,17 @@ def _set_properties(self): Update the base_hook accordingly. """ hook: Dict = {} + keys_to_delete = [] for full_key in self.base_hook: key = full_key.split(":")[0] if hook.get(key): continue + if key in PROPERTIES_TO_DELETE: + keys_to_delete.append(key) if (prop := self._get_property(key)) is not None: hook[key] = prop + for key in keys_to_delete: + del hook[key] self.base_hook = hook @@ -134,7 +145,9 @@ def join_files(files: Set[Path], separator: str = "|") -> str: Returns: str: The joined string. """ - return separator.join(map(str, files)) + return separator.join( + str(file) if Path(file).is_file() else f"{str(file)}{os.sep}" for file in files + ) def safe_update_hook_args(hook: Dict, value: Any) -> None: diff --git a/demisto_sdk/commands/pre_commit/hooks/ruff.py b/demisto_sdk/commands/pre_commit/hooks/ruff.py index 18d6491c39..61ea80a64f 100644 --- a/demisto_sdk/commands/pre_commit/hooks/ruff.py +++ b/demisto_sdk/commands/pre_commit/hooks/ruff.py @@ -45,6 +45,12 @@ def prepare_hook( 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]) + hook["files"] = join_files( + { + file + for file in python_version_to_files[python_version] + if file.suffix == ".py" + } + ) self.hooks.append(hook) diff --git a/demisto_sdk/commands/pre_commit/hooks/system.py b/demisto_sdk/commands/pre_commit/hooks/system.py new file mode 100644 index 0000000000..09c16999df --- /dev/null +++ b/demisto_sdk/commands/pre_commit/hooks/system.py @@ -0,0 +1,23 @@ +import sys +from pathlib import Path + +from demisto_sdk.commands.pre_commit.hooks.hook import ( + Hook, +) + + +class SystemHook(Hook): + def prepare_hook(self, **kwargs): + """ + Prepares the Validate or the Format hook. + In case of nightly mode and all files, runs validate/format with the --all flag, (nightly mode is not supported on specific files). + 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: + files_to_run (Optional[Iterable[Path]]): The input files to validate. Defaults to None. + """ + if "entry" in self.base_hook: + entry = self.base_hook["entry"] + bin_path = Path(sys.executable).parent + self.base_hook["entry"] = f"{bin_path}/{entry}" + self.hooks.insert(self.hook_index, self.base_hook) diff --git a/demisto_sdk/commands/pre_commit/hooks/validate_format.py b/demisto_sdk/commands/pre_commit/hooks/validate_format.py index ad84795acf..547ad9305a 100644 --- a/demisto_sdk/commands/pre_commit/hooks/validate_format.py +++ b/demisto_sdk/commands/pre_commit/hooks/validate_format.py @@ -18,9 +18,9 @@ def prepare_hook(self, files_to_run: Optional[Iterable[Path]], **kwargs): Args: files_to_run (Optional[Iterable[Path]]): The input files to validate. Defaults to None. """ - if self.mode and self.mode.lower() == "nightly" and self.all_files: + if self.all_files: safe_update_hook_args(self.base_hook, "-a") - elif self.input_mode or self.all_files: + elif self.input_mode: safe_update_hook_args(self.base_hook, "-i") self.base_hook["args"].append(join_files(files_to_run, ",")) else: diff --git a/demisto_sdk/commands/pre_commit/pre_commit_command.py b/demisto_sdk/commands/pre_commit/pre_commit_command.py index f4667ec320..56593c5d23 100644 --- a/demisto_sdk/commands/pre_commit/pre_commit_command.py +++ b/demisto_sdk/commands/pre_commit/pre_commit_command.py @@ -2,11 +2,13 @@ import multiprocessing import os import re +import shutil import subprocess import sys from collections import defaultdict -from dataclasses import dataclass -from functools import cached_property +from dataclasses import dataclass, field +from functools import cached_property, partial +from multiprocessing.pool import ThreadPool from pathlib import Path from typing import Dict, Iterable, List, Optional, Set, Tuple @@ -16,14 +18,15 @@ DEFAULT_PYTHON2_VERSION, DEFAULT_PYTHON_VERSION, INTEGRATIONS_DIR, + PACKS_FOLDER, SCRIPTS_DIR, ) from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH, PYTHONPATH +from demisto_sdk.commands.common.cpu_count import cpu_count from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.tools import ( get_file_or_remote, - get_last_remote_release_version, get_remote_file, string_to_bool, write_dict, @@ -38,13 +41,17 @@ from demisto_sdk.commands.pre_commit.hooks.pycln import PyclnHook from demisto_sdk.commands.pre_commit.hooks.ruff import RuffHook from demisto_sdk.commands.pre_commit.hooks.sourcery import SourceryHook +from demisto_sdk.commands.pre_commit.hooks.system import SystemHook from demisto_sdk.commands.pre_commit.hooks.validate_format import ValidateFormatHook IS_GITHUB_ACTIONS = string_to_bool(os.getenv("GITHUB_ACTIONS"), False) PRECOMMIT_TEMPLATE_NAME = ".pre-commit-config_template.yaml" PRECOMMIT_TEMPLATE_PATH = CONTENT_PATH / PRECOMMIT_TEMPLATE_NAME -PRECOMMIT_PATH = CONTENT_PATH / ".pre-commit-config-content.yaml" +PRECOMMIT_FOLDER = CONTENT_PATH / ".pre-commit" +PRECOMMIT_CONFIG = PRECOMMIT_FOLDER / "config" +PRECOMMIT_CONFIG_MAIN_PATH = PRECOMMIT_CONFIG / ".pre-commit-config-main.yaml" +PRECOMMIT_DOCKER_CONFIGS = PRECOMMIT_CONFIG / "docker" SOURCERY_CONFIG_PATH = CONTENT_PATH / ".sourcery.yaml" SKIPPED_HOOKS = {"format", "validate", "secrets"} @@ -56,10 +63,10 @@ "check-yaml", "check-ast", "check-merge-conflict", - "run-unit-tests", "validate", "format", "pylint-in-docker", + "pytest-in-docker", } @@ -67,18 +74,25 @@ class PreCommitRunner: """This class is responsible of running pre-commit hooks.""" - input_mode: bool + input_files: Optional[List[Path]] all_files: bool - mode: str - python_version_to_files_with_objects: Dict[ + mode: Optional[str] + language_version_to_files_with_objects: Dict[ str, Set[Tuple[Path, Optional[IntegrationScript]]] ] - demisto_sdk_commit_hash: str + run_hook: Optional[str] = None + skipped_hooks: Set[str] = field(default_factory=set) + run_docker_hooks: bool = True def __post_init__(self): """ We initialize the hooks and all_files for later use. """ + shutil.rmtree(PRECOMMIT_FOLDER, ignore_errors=True) + PRECOMMIT_FOLDER.mkdir() + PRECOMMIT_CONFIG.mkdir() + PRECOMMIT_DOCKER_CONFIGS.mkdir() + self.precommit_template = get_file_or_remote(PRECOMMIT_TEMPLATE_PATH) remote_config_file = get_remote_file(str(PRECOMMIT_TEMPLATE_PATH)) if remote_config_file and remote_config_file != self.precommit_template: @@ -89,16 +103,8 @@ def __post_init__(self): raise TypeError( f"Pre-commit template in {PRECOMMIT_TEMPLATE_PATH} is not a dictionary." ) - if self.mode: - logger.info( - f"[yellow]Running pre-commit hooks in `{self.mode}` mode.[/yellow]" - ) - # changes the demisto-sdk revision to the latest release version (or the debug commit hash) - # to debug, modify the DEMISTO_SDK_COMMIT_HASH_DEBUG variable to your demisto-sdk commit hash - self._get_repos(self.precommit_template)[ - "https://github.com/demisto/demisto-sdk" - ]["rev"] = self.demisto_sdk_commit_hash self.hooks = self._get_hooks(self.precommit_template) + self.hooks_need_docker = self._hooks_need_docker() @cached_property def files_to_run_with_objects( @@ -106,7 +112,7 @@ def files_to_run_with_objects( ) -> Set[Tuple[Path, Optional[IntegrationScript]]]: return set( itertools.chain.from_iterable( - self.python_version_to_files_with_objects.values() + self.language_version_to_files_with_objects.values() ) ) @@ -114,11 +120,19 @@ def files_to_run_with_objects( def files_to_run(self) -> Set[Path]: return {file for file, _ in self.files_to_run_with_objects} + @cached_property + def language_to_files(self) -> Dict[str, Set[Path]]: + return { + version: {path for path, _ in paths_with_objects} + for version, paths_with_objects in self.language_version_to_files_with_objects.items() + } + @cached_property def python_version_to_files(self) -> Dict[str, Set[Path]]: return { version: {path for path, _ in paths_with_objects} - for version, paths_with_objects in self.python_version_to_files_with_objects.items() + for version, paths_with_objects in self.language_version_to_files_with_objects.items() + if version not in {"javascript", "powershell"} } @staticmethod @@ -130,15 +144,23 @@ def _get_repos(pre_commit_config: dict) -> dict: def _get_hooks(self, pre_commit_config: dict) -> dict: hooks = {} - for repo in pre_commit_config["repos"]: new_hooks = [] for hook in repo["hooks"]: - if not Hook.get_property(hook, self.mode, "skip"): + if not self.run_docker_hooks and hook["id"].endswith("in-docker"): + continue + if (self.run_hook and hook["id"] in self.run_hook) or ( + not self.run_hook + and hook["id"] not in self.skipped_hooks + and not Hook.get_property(hook, self.mode, "skip") + ): + needs = Hook.get_property(hook, self.mode, "needs") + if needs and any(need not in hooks for need in needs): + continue new_hooks.append(hook) hooks[hook["id"]] = {"repo": repo, "hook": hook} - repo["hooks"] = new_hooks + repo["hooks"] = new_hooks return hooks @@ -164,15 +186,12 @@ def exclude_python2_of_non_supported_hooks(self) -> None: else: hook["hook"]["exclude"] = join_files_string - def prepare_hooks( - self, - run_docker_hooks, - ) -> None: + def prepare_hooks(self, dry_run: bool) -> None: hooks = self.hooks kwargs = { "mode": self.mode, "all_files": self.all_files, - "input_mode": self.input_mode, + "input_mode": bool(self.input_files), } if "pycln" in hooks: PyclnHook(**hooks.pop("pycln"), **kwargs).prepare_hook(PYTHONPATH) @@ -190,61 +209,269 @@ def prepare_hooks( ) if "validate" in hooks: ValidateFormatHook(**hooks.pop("validate"), **kwargs).prepare_hook( - self.files_to_run + self.input_files ) if "format" in hooks: ValidateFormatHook(**hooks.pop("format"), **kwargs).prepare_hook( - self.files_to_run + self.input_files ) [ - DockerHook(**hook, **kwargs).prepare_hook( + DockerHook(**hooks.pop(hook_id), **kwargs).prepare_hook( files_to_run_with_objects=self.files_to_run_with_objects, - run_docker_hooks=run_docker_hooks, + dry_run=dry_run, ) - for hook_id, hook in hooks.items() + for hook_id in hooks.copy() if hook_id.endswith("in-docker") ] - hooks_without_docker = [ - hook for hook_id, hook in hooks.items() if not hook_id.endswith("in-docker") + # iterate the rest of the hooks + for hook_id in hooks.copy(): + # this is used to handle the mode property correctly + Hook(**hooks.pop(hook_id), **kwargs).prepare_hook() + # get the hooks again because we want to get all the hooks, including the once that already prepared + hooks = self._get_hooks(self.precommit_template) + system_hooks = [ + hook_id + for hook_id, hook in hooks.items() + if hook["hook"].get("language") == "system" + ] + for hook_id in system_hooks.copy(): + SystemHook(**hooks.pop(hook_id), **kwargs).prepare_hook() + + def _hooks_need_docker(self) -> Set[str]: + """ + Get all the hook ids that needs docker based on the "needs" property + """ + return { + hook_id + for hook_id, hook in self.hooks.items() + if (needs := hook["hook"].pop("needs", None)) + and any("in-docker" in need for need in needs) + } + + def _get_docker_and_no_docker_hooks( + self, local_repo: dict + ) -> Tuple[List[dict], List[dict]]: + """This function separates the docker and no docker hooks of a local repo + + Args: + local_repo (dict): The local repo + + Returns: + Tuple[List[dict], List[dict]]: The first item is the list of docker hooks, the second is the list of no docker hooks + """ + local_repo_hooks = local_repo["hooks"] + docker_hooks = [hook for hook in local_repo_hooks if "in-docker" in hook["id"]] + no_docker_hooks = [ + hook for hook in local_repo_hooks if "in-docker" not in hook["id"] ] - for hook in hooks_without_docker: - Hook(**hook, **kwargs).prepare_hook() + return docker_hooks, no_docker_hooks + + def run_hooks( + self, + index: Optional[int], + precommit_env: dict, + verbose: bool = False, + stdout: Optional[int] = subprocess.PIPE, + ): + """This function runs the pre-commit process and waits until finished. + We run this function in multithread. + + Args: + index (Optional[int]): The index of the docker hook. if None, runs main pre-commit config + precommit_env (dict): The pre-commit environment variables + verbose (bool, optional): Whether print verbose output. Defaults to False. + stdout (Optional[int], optional): The way to handle stdout. Defaults to subprocess.PIPE. + + Returns: + int: return code - 0 if hooks passed, 1 if failed + """ + if index is None: + process = self._run_pre_commit_process( + PRECOMMIT_CONFIG_MAIN_PATH, precommit_env, verbose, stdout + ) + else: + process = self._run_pre_commit_process( + PRECOMMIT_DOCKER_CONFIGS / f"pre-commit-config-docker-{index}.yaml", + precommit_env, + verbose, + stdout, + ) + if process.stdout: + logger.info(process.stdout) + if process.stderr: + logger.error(process.stderr) + return process.returncode + + def _filter_hooks_need_docker(self, repos: dict) -> dict: + """ + This filters the pre-commit config file the hooks that needed docker, so we will be able to execute them after the docker hooks are finished + """ + full_hooks_need_docker = {} + for repo, repo_dict in repos.items(): + hooks = [] + for hook in repo_dict["hooks"]: + if hook["id"] not in self.hooks_need_docker: + hooks.append(hook) + else: + full_hooks_need_docker[hook["id"]] = { + "repo": repo_dict, + "hook": hook, + } + repo_dict["hooks"] = hooks + return full_hooks_need_docker + + def _update_hooks_needs_docker(self, hooks_needs_docker: dict): + """ + This is to populate the pre-commit config file only for hooks that needs docker + This is needed because we need to execute this after all docker hooks are finished + """ + self.precommit_template["repos"] = [] + for _, hook in hooks_needs_docker.items(): + repos = {repo["repo"] for repo in self.precommit_template["repos"]} + repo_in_hook = hook["repo"]["repo"] + if repo_in_hook not in repos: + hook["repo"]["hooks"] = [] + self.precommit_template["repos"].append(hook["repo"]) + repo = self.precommit_template["repos"][-1] + else: + repo = next( + repo + for repo in self.precommit_template["repos"] + if repo["repo"] == repo_in_hook + ) + repo["hooks"].append(hook["hook"]) + + def _run_pre_commit_process( + self, + path: Path, + precommit_env: dict, + verbose: bool, + stdout=None, + command: Optional[List[str]] = None, + ) -> subprocess.CompletedProcess: + """Runs a process of pre-commit + + Args: + path (Path): Pre commit path + precommit_env (dict): Environment variables set on pre-commit + verbose (bool): whether to print verbose output + stdout (optional): use `subprocess.PIPE` to capture stdout. Use None to print it. Defaults to None. + command (Optional[List[str]], optional): The pre-commit command to run. Defaults to None. + + Returns: + _type_: _description_ + """ + if command is None: + command = ["run", "-a"] + return subprocess.run( + list( + filter( + None, + [ + sys.executable, + "-m", + "pre_commit", + *command, + "-c", + str(path), + "-v" if verbose and "run" in command else "", + ], + ) + ), + env=precommit_env, + cwd=CONTENT_PATH, + stdout=stdout, + stderr=stdout, + universal_newlines=True, + ) def run( + self, precommit_env: dict, verbose: bool, show_diff_on_failure: bool + ) -> int: + if self.mode: + logger.info( + f"[yellow]Running pre-commit hooks in `{self.mode}` mode.[/yellow]" + ) + if self.run_hook: + logger.info(f"[yellow]Running hook {self.run_hook}[/yellow]") + repos = self._get_repos(self.precommit_template) + local_repo = repos["local"] + docker_hooks, no_docker_hooks = self._get_docker_and_no_docker_hooks(local_repo) + local_repo["hooks"] = no_docker_hooks + full_hooks_need_docker = self._filter_hooks_need_docker(repos) + + num_processes = cpu_count() + logger.info(f"Pre-Commit will use {num_processes} processes") + write_dict(PRECOMMIT_CONFIG_MAIN_PATH, self.precommit_template) + # first, run the hooks without docker hooks + stdout = subprocess.PIPE if docker_hooks else None + self._run_pre_commit_process( + PRECOMMIT_CONFIG_MAIN_PATH, + precommit_env, + verbose, + command=["install-hooks"], + ) + for i, hook in enumerate(docker_hooks): + self.precommit_template["repos"] = [local_repo] + local_repo["hooks"] = [hook] + path = PRECOMMIT_DOCKER_CONFIGS / f"pre-commit-config-docker-{i}.yaml" + write_dict(path, data=self.precommit_template) + + # the threads will run in separate process and will wait for completion + with ThreadPool(num_processes) as pool: + results = pool.map( + partial( + self.run_hooks, + precommit_env=precommit_env, + verbose=verbose, + stdout=stdout, + ), + [None] + list(range(len(docker_hooks))), + ) + return_code = int(any(results)) + if self.hooks_need_docker: + # run hooks that needs docker after all the docker hooks finished + self._update_hooks_needs_docker(full_hooks_need_docker) + path = PRECOMMIT_CONFIG_MAIN_PATH.with_name( + f"{PRECOMMIT_CONFIG_MAIN_PATH.stem}-needs.yaml" + ) + write_dict(path, self.precommit_template) + process_needs_docker = self._run_pre_commit_process( + path, precommit_env, verbose=verbose + ) + + return_code = return_code or process_needs_docker.returncode + + if return_code and show_diff_on_failure: + logger.info( + "Pre-Commit changed the following. If you experience this in CI, please run `demisto-sdk pre-commit`" + ) + git_diff = subprocess.run( + ["git", "--no-pager", "diff", "--no-ext-diff"], + stdout=subprocess.PIPE, + universal_newlines=True, + ) + logger.info(git_diff.stdout) + return return_code + + def prepare_and_run( self, - unit_test: bool = False, - skip_hooks: Optional[List[str]] = None, - validate: bool = False, - format: bool = False, - secrets: bool = False, verbose: bool = False, show_diff_on_failure: bool = False, exclude_files: Optional[Set[Path]] = None, dry_run: bool = False, - run_docker_hooks: bool = True, ) -> int: + ret_val = 0 precommit_env = os.environ.copy() - skipped_hooks: set = SKIPPED_HOOKS - 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-pytest-reports") - if validate and "validate" in skipped_hooks: - skipped_hooks.remove("validate") - if format and "format" in skipped_hooks: - skipped_hooks.remove("format") - if secrets and "secrets" in skipped_hooks: - skipped_hooks.remove("secrets") - precommit_env["SKIP"] = ",".join(sorted(skipped_hooks)) precommit_env["PYTHONPATH"] = ":".join(str(path) for path in PYTHONPATH) # The PYTHONPATH should be the same as the PYTHONPATH, but without the site-packages because MYPY does not support it precommit_env["MYPYPATH"] = ":".join( str(path) for path in sorted(PYTHONPATH) if "site-packages" not in str(path) ) precommit_env["DEMISTO_SDK_CONTENT_PATH"] = str(CONTENT_PATH) - + precommit_env["SYSTEMD_COLORS"] = "1" # for colorful output + precommit_env["PRE_COMMIT_COLOR"] = "always" self.exclude_python2_of_non_supported_hooks() for ( @@ -258,7 +485,7 @@ def run( f"Running pre-commit with Python {python_version} on {changed_files_string}" ) - self.prepare_hooks(run_docker_hooks) + self.prepare_hooks(dry_run) if self.all_files: self.precommit_template[ "exclude" @@ -266,42 +493,17 @@ def run( else: self.precommit_template["files"] = join_files(self.files_to_run) - write_dict(PRECOMMIT_PATH, data=self.precommit_template) - if dry_run: + write_dict(PRECOMMIT_CONFIG_MAIN_PATH, data=self.precommit_template) logger.info( - "Dry run, skipping pre-commit.\nConfig file saved to .pre-commit-config-content.yaml" + f"Dry run, skipping pre-commit.\nConfig file saved to {PRECOMMIT_CONFIG_MAIN_PATH}" ) return ret_val - response = subprocess.run( - list( - filter( - None, - [ - sys.executable, - "-m", - "pre_commit", - "run", - "-a", - "-c", - str(PRECOMMIT_PATH), - "--show-diff-on-failure" if show_diff_on_failure else "", - "-v" if verbose else "", - ], - ) - ), - env=precommit_env, - cwd=CONTENT_PATH, - ) - if response.returncode: - ret_val = 1 - - # remove the config file in the end of the flow - PRECOMMIT_PATH.unlink(missing_ok=True) + ret_val = self.run(precommit_env, verbose, show_diff_on_failure) return ret_val -def group_by_python_version( +def group_by_language( files: Set[Path], ) -> Tuple[Dict[str, Set[Tuple[Path, Optional[IntegrationScript]]]], Set[Path]]: """This function groups the files to run pre-commit on by the python version. @@ -313,14 +515,17 @@ def group_by_python_version( Exception: If invalid files were given. Returns: - Dict[str, set]: The files grouped by their python version. + Dict[str, set]: The files grouped by their python version, and a set of excluded paths """ integrations_scripts_mapping = defaultdict(set) infra_files = [] for file in files: if file.is_dir(): continue - if set(file.parts) & {INTEGRATIONS_DIR, SCRIPTS_DIR}: + if ( + set(file.parts) & {INTEGRATIONS_DIR, SCRIPTS_DIR} + and file.parts[0] == PACKS_FOLDER # this is relative path so it works + ): find_path_index = ( i + 1 for i, part in enumerate(file.parts) @@ -331,11 +536,13 @@ def group_by_python_version( code_file_path = CONTENT_PATH / Path( *file.parts[: next(find_path_index) + 1] ) + if not code_file_path.is_dir(): + continue integrations_scripts_mapping[code_file_path].add(file) else: infra_files.append(file) - python_versions_to_files: Dict[str, Set] = defaultdict(set) + language_to_files: Dict[str, Set] = defaultdict(set) with multiprocessing.Pool() as pool: integrations_scripts = pool.map( BaseContent.from_path, integrations_scripts_mapping.keys() @@ -348,6 +555,8 @@ def group_by_python_version( ): continue if integration_script.deprecated: + # we exclude deprecate integrations and scripts from pre-commit. + # the reason we maintain this set is for performance when running with --all-files. It is much faster to exclude. if integration_script.is_unified: exclude_integration_script.add( integration_script.path.relative_to(CONTENT_PATH) @@ -361,16 +570,10 @@ def group_by_python_version( code_file_path = integration_script.path.parent if python_version := integration_script.python_version: version = Version(python_version) - python_version_string = f"{version.major}.{version.minor}" + language = f"{version.major}.{version.minor}" else: - # Skip cases of powershell scripts - exclude_integration_script.add( - integration_script.path.relative_to(CONTENT_PATH) - ) - continue - python_versions_to_files[ - python_version_string or DEFAULT_PYTHON2_VERSION - ].update( + language = integration_script.type + language_to_files[language].update( { (path, integration_script) for path in integrations_scripts_mapping[code_file_path] @@ -379,15 +582,15 @@ def group_by_python_version( ) if infra_files: - python_versions_to_files[DEFAULT_PYTHON_VERSION].update( + language_to_files[DEFAULT_PYTHON_VERSION].update( [(infra, None) for infra in infra_files] ) if exclude_integration_script: logger.info( - f"Skipping deprecated or powershell integrations or scripts: {join_files(exclude_integration_script, ', ')}" + f"Skipping deprecated integrations or scripts: {join_files(exclude_integration_script, ', ')}" ) - return python_versions_to_files, exclude_integration_script + return language_to_files, exclude_integration_script def pre_commit_manager( @@ -397,17 +600,16 @@ def pre_commit_manager( git_diff: bool = False, all_files: bool = False, mode: str = "", - unit_test: bool = False, skip_hooks: Optional[List[str]] = None, validate: bool = False, format: bool = False, secrets: bool = False, verbose: bool = False, show_diff_on_failure: bool = False, - sdk_ref: Optional[str] = None, dry_run: bool = False, run_docker_hooks: bool = True, -) -> Optional[int]: + run_hook: Optional[str] = None, +) -> int: """Run pre-commit hooks . Args: @@ -417,7 +619,6 @@ def pre_commit_manager( 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 (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. verbose (bool, optional): Whether run pre-commit in verbose mode. Defaults to False. @@ -440,42 +641,36 @@ def pre_commit_manager( ) if not files_to_run: logger.info("No files were changed, skipping pre-commit.") - return None - - files_to_run_string = ", ".join( - sorted((str(changed_path) for changed_path in files_to_run)) - ) - - # This is the files that pre-commit received, but in fact it will run on files returned from group_by_python_version - logger.info(f"pre-commit received the following files: {files_to_run_string}") + return 0 - if not sdk_ref: - sdk_ref = f"v{get_last_remote_release_version()}" - python_version_to_files_with_objects, exclude_files = group_by_python_version( - files_to_run - ) - if not python_version_to_files_with_objects: + language_to_files_with_objects, exclude_files = group_by_language(files_to_run) + if not language_to_files_with_objects: logger.info("No files to run pre-commit on, skipping pre-commit.") - return None + return 0 + + skipped_hooks: set = SKIPPED_HOOKS + skipped_hooks.update(set(skip_hooks or ())) + if validate and "validate" in skipped_hooks: + skipped_hooks.remove("validate") + if format and "format" in skipped_hooks: + skipped_hooks.remove("format") + if secrets and "secrets" in skipped_hooks: + skipped_hooks.remove("secrets") pre_commit_runner = PreCommitRunner( - bool(input_files), + list(input_files) if input_files else None, all_files, mode, - python_version_to_files_with_objects, - sdk_ref, + language_to_files_with_objects, + run_hook, + skipped_hooks, + run_docker_hooks, ) - return pre_commit_runner.run( - unit_test, - skip_hooks, - validate, - format, - secrets, + return pre_commit_runner.prepare_and_run( verbose, show_diff_on_failure, exclude_files, dry_run, - run_docker_hooks, ) @@ -514,6 +709,13 @@ def preprocess_files( py_file_path = file.with_suffix(".py") if py_file_path.exists(): files_to_run.add(py_file_path) + if file.suffix in (".py", ".ps1"): + if file.suffix == ".py": + test_file = file.with_name(f"{file.stem}_test.py") + else: + test_file = file.with_name(f"{file.stem}.Tests.ps1") + if test_file.exists(): + files_to_run.add(test_file) # convert to relative file to content path relative_paths = { 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 793bab25f4..0e5ab85bd9 100644 --- a/demisto_sdk/commands/pre_commit/tests/generic_docker_test.py +++ b/demisto_sdk/commands/pre_commit/tests/generic_docker_test.py @@ -11,7 +11,7 @@ from demisto_sdk.commands.pre_commit.tests.pre_commit_test import create_hook -@dataclass +@dataclass(frozen=True) class Obj: path: Path = Path("somefile") object_id: str = "id1" @@ -78,7 +78,7 @@ def test_moded_properties(mocker, mode, expected_text): "demisto_sdk.commands.pre_commit.hooks.docker.devtest_image", return_value="devtestimg", ) - raw_hook = create_hook({"args": []}) + raw_hook = create_hook({"args": [], "language": "docker"}) raw_hook["hook"]["args"] = ["i am some argument"] raw_hook["hook"]["args:nightly"] = ["i am the nightly args"] 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 099c6e9dfe..e49c1cdd86 100644 --- a/demisto_sdk/commands/pre_commit/tests/pre_commit_test.py +++ b/demisto_sdk/commands/pre_commit/tests/pre_commit_test.py @@ -1,4 +1,5 @@ import itertools +from dataclasses import dataclass from pathlib import Path import pytest @@ -10,11 +11,12 @@ 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.system import SystemHook from demisto_sdk.commands.pre_commit.hooks.validate_format import ValidateFormatHook from demisto_sdk.commands.pre_commit.pre_commit_command import ( PYTHON2_SUPPORTED_HOOKS, GitUtil, - group_by_python_version, + group_by_language, preprocess_files, subprocess, ) @@ -40,8 +42,24 @@ def create_hook(hook: dict): return repo_and_hook -@pytest.mark.parametrize("is_test", [True, False]) -def test_config_files(mocker, repo: Repo, is_test: bool): +@dataclass +class MockProcess: + + returncode = 0 + stdout = "finished" + stderr = "" + + def poll(self): + return self.returncode + + def wait(self): + return self.returncode + + def communicate(self): + return "", "" + + +def test_config_files(mocker, repo: Repo): """ Given: A repository with different scripts and integration of different python versions @@ -52,14 +70,32 @@ def test_config_files(mocker, repo: Repo, is_test: bool): Then: Categorize the scripts and integration by python version, and make sure that pre-commit configuration is created for each """ + mocker.patch.object(DockerHook, "__init__", return_value=None) + mocker.patch.object( + DockerHook, + "prepare_hook", + return_value=[{"id": "run-in-docker"}], + ) mocker.patch.object( pre_commit_command, "PRECOMMIT_TEMPLATE_PATH", - TEST_DATA_PATH / ".pre-commit-config_template.yaml", + TEST_DATA_PATH / ".pre-commit-config_template-test.yaml", ) pack1 = repo.create_pack("Pack1") mocker.patch.object(pre_commit_command, "CONTENT_PATH", Path(repo.path)) - + mocker.patch.object( + pre_commit_command, + "PRECOMMIT_CONFIG_MAIN_PATH", + Path(repo.path) / ".pre-commit-config.yaml", + ) + mocker.patch.object( + pre_commit_command, + "PRECOMMIT_DOCKER_CONFIGS", + Path(repo.path) / "docker-config", + ) + mocker.patch.object( + pre_commit_command, "PRECOMMIT_CONFIG", Path(repo.path) / "config" + ) integration1 = pack1.create_integration( "integration1", docker_image="demisto/python3:3.9.1.14969" ) @@ -77,7 +113,8 @@ def test_config_files(mocker, repo: Repo, is_test: bool): incident_field = pack1.create_incident_field("incident_field") classifier = pack1.create_classifier("classifier") mocker.patch.object(yaml, "dump", side_effect=lambda *args: []) - mock_subprocess = mocker.patch.object(subprocess, "run") + mocker.patch.object(subprocess, "run", return_value=MockProcess()) + relative_paths = { path.relative_to(repo.path) for path in Path(pack1.path).rglob("*") @@ -92,7 +129,7 @@ def test_config_files(mocker, repo: Repo, is_test: bool): files_to_run = preprocess_files([Path(pack1.path)]) assert files_to_run == relative_paths - python_version_to_files, _ = group_by_python_version(files_to_run) + python_version_to_files, _ = group_by_language(files_to_run) pre_commit = pre_commit_command.PreCommitRunner( None, None, None, python_version_to_files, "" ) @@ -122,17 +159,10 @@ def test_config_files(mocker, repo: Repo, is_test: bool): not in pre_commit.python_version_to_files["3.10"] ) - pre_commit.run(unit_test=is_test) - - assert mock_subprocess.call_count == 1 - - tests_we_should_skip = {"format", "validate", "secrets"} - 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-pytest-reports") - for m in mock_subprocess.call_args_list: - assert set(m.kwargs["env"]["SKIP"].split(",")) == tests_we_should_skip + pre_commit.prepare_and_run() + assert (Path(repo.path) / ".pre-commit-config.yaml").exists() + assert list((Path(repo.path) / "docker-config").iterdir()) + assert (Path(repo.path) / ".pre-commit-config-needs.yaml").exists() def test_mypy_hooks(): @@ -240,8 +270,8 @@ def test_validate_format_hook_all_files(): ) hook_args = validate_format_hook["repo"]["hooks"][0]["args"] - assert "-a" not in hook_args - assert "-i" in hook_args + assert "-a" in hook_args + assert "-i" not in hook_args class TestPreprocessFiles: @@ -348,7 +378,7 @@ def test_exclude_python2_of_non_supported_hooks(mocker, repo: Repo): mocker.patch.object( pre_commit_command, "PRECOMMIT_TEMPLATE_PATH", - TEST_DATA_PATH / ".pre-commit-config_template.yaml", + TEST_DATA_PATH / ".pre-commit-config_template-test.yaml", ) mocker.patch.object(pre_commit_command, "CONTENT_PATH", Path(repo.path)) mocker.patch.object(pre_commit_command, "logger") @@ -366,7 +396,9 @@ def test_exclude_python2_of_non_supported_hooks(mocker, repo: Repo): for hook in pre_commit_runner.hooks.values(): if hook["hook"]["id"] in PYTHON2_SUPPORTED_HOOKS: - assert hook["hook"].get("exclude") is None + assert not hook["hook"].get("exclude") or "file1.py" not in hook[ + "hook" + ].get("exclude") else: assert "file1.py" in hook["hook"]["exclude"] @@ -466,25 +498,6 @@ def test_filter_files_matching_hook_config(hook, expected_result): ) -def test_no_docker_flag_docker_hook(): - """ - Given: - run_docker flag (--no-docker) is given. - When: - running pre-commit command. - Then: - Do not add the docker hook to the list of hooks. - """ - file_path = Path("SomeFile.py") - docker_hook = create_hook({"args": ["test"]}) - kwargs = {"mode": None, "all_files": False, "input_mode": True} - DockerHook(**docker_hook, **kwargs).prepare_hook( - files_to_run_with_objects=[file_path], run_docker_hooks=False - ) - - assert len(docker_hook["repo"]["hooks"]) == 0 - - def test_skip_hook_with_mode(mocker): """ Given: @@ -499,7 +512,7 @@ def test_skip_hook_with_mode(mocker): mocker.patch.object( pre_commit_command, "PRECOMMIT_TEMPLATE_PATH", - TEST_DATA_PATH / ".pre-commit-config_template.yaml", + TEST_DATA_PATH / ".pre-commit-config_template-test.yaml", ) python_version_to_files = {"2.7": {"file1.py"}, "3.8": {"file2.py"}} pre_commit_runner = pre_commit_command.PreCommitRunner( @@ -514,3 +527,27 @@ def test_skip_hook_with_mode(mocker): hook.get("id") for hook in repos["https://github.com/demisto/demisto-sdk"]["hooks"] } + + +def test_system_hooks(): + """ + Given: + hook with `system` language + + When: + running pre-commit command. + + Then: + The hook entry is updated with the path to the python interpreter that is running. + """ + import sys + + Path(sys.executable).parent + system_hook = create_hook( + {"args": [], "entry": "demisto-sdk", "language": "system"} + ) + SystemHook(**system_hook).prepare_hook() + assert ( + system_hook["repo"]["hooks"][0]["entry"] + == f"{Path(sys.executable).parent}/demisto-sdk" + ) diff --git a/demisto_sdk/commands/pre_commit/tests/test_data/.pre-commit-config_template.yaml b/demisto_sdk/commands/pre_commit/tests/test_data/.pre-commit-config_template-test.yaml similarity index 75% rename from demisto_sdk/commands/pre_commit/tests/test_data/.pre-commit-config_template.yaml rename to demisto_sdk/commands/pre_commit/tests/test_data/.pre-commit-config_template-test.yaml index d18893b6e2..1c2ff2d784 100644 --- a/demisto_sdk/commands/pre_commit/tests/test_data/.pre-commit-config_template.yaml +++ b/demisto_sdk/commands/pre_commit/tests/test_data/.pre-commit-config_template-test.yaml @@ -46,6 +46,18 @@ repos: pass_filenames: false skip:nightly: true + - id: pylint-in-docker + name: pylint-in-docker + description: Run pylint on the code in content packs + docker_image: from-yml + entry: pylint + files: Packs\/.*\.py$ + exclude: _test\.py|.vulture_whitelist\.py|test_data + args: ['--ignore=demistomock.py,CommonServerPython.py,CommonServerUserPython.py,conftest.py,.venv', '-E', '--disable=bad-option-value,unsubscriptable-object', '-d duplicate-string-formatting-argument', "--msg-template='{path}:{line}:{column}: {msg_id} {obj}: {msg}'", '--generated-members=requests.packages.urllib3,requests.codes.ok'] + config_file_arg: + arg_name: '--rcfile' + file_name: '.pylintrc' + - repo: https://github.com/demisto/demisto-sdk rev: "" hooks: @@ -78,6 +90,10 @@ repos: - id: should_be_skipped # only for the unit test skip: true + - id: hook_needs_docker + needs: + - pylint-in-docker + - repo: https://github.com/sourcery-ai/sourcery rev: "v1.6.0" hooks: diff --git a/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py b/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py deleted file mode 100644 index 0b4db0ae76..0000000000 --- a/demisto_sdk/commands/run_unit_tests/unit_tests_runner.py +++ /dev/null @@ -1,150 +0,0 @@ -import os -import traceback -from pathlib import Path -from typing import List - -from junitparser import JUnitXml - -import demisto_sdk.commands.common.docker_helper as docker_helper -from demisto_sdk.commands.common.constants import TYPE_JS -from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH, PYTHONPATH -from demisto_sdk.commands.common.logger import logger -from demisto_sdk.commands.content_graph.objects.base_content import BaseContent -from demisto_sdk.commands.content_graph.objects.integration_script import ( - IntegrationScript, -) -from demisto_sdk.commands.lint.helpers import stream_docker_container_output - -DOCKER_PYTHONPATH = [ - f"/content/{path.relative_to(CONTENT_PATH)}" - for path in PYTHONPATH - if path.is_relative_to(CONTENT_PATH) -] - -DEFAULT_DOCKER_IMAGE = "demisto/python:1.3-alpine" - -PYTEST_COMMAND = "python -m pytest . -v --rootdir=/content --override-ini='asyncio_mode=auto' --override-ini='junit_family=xunit1' --junitxml=.report_pytest.xml --cov-report= --cov=." -PWSH_COMMAND = "pwsh -Command Invoke-Pester -Configuration '@{Run=@{Exit=$true}; Output=@{Verbosity=\"Detailed\"}}'" -TEST_REQUIREMENTS_DIR = Path(__file__).parent.parent / "lint" / "resources" - - -NO_TESTS_COLLECTED = 5 - - -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() - exit_code = 0 - for filename in file_paths: - integration_script = BaseContent.from_path(Path(filename)) - if not isinstance(integration_script, IntegrationScript): - logger.warning(f"Skipping {filename} as it is not a content item.") - continue - - if integration_script.type == TYPE_JS: - logger.info( - f"Skipping tests for '{integration_script.name}' since it is a JavaScript integration/script" - ) - continue - - relative_integration_script_path = integration_script.path.relative_to( - CONTENT_PATH - ) - - if (test_data_dir := (integration_script.path.parent / "test_data")).exists(): - (test_data_dir / "__init__.py").touch() - - working_dir = f"/content/{relative_integration_script_path.parent}" - docker_images = [integration_script.docker_image or DEFAULT_DOCKER_IMAGE] - if os.getenv("CONTENT_GITLAB_CI"): - docker_images = [ - f"docker-io.art.code.pan.run/{docker_image}" - for docker_image in docker_images - ] - logger.debug(f"{docker_images=}") - for docker_image in docker_images: - try: - test_docker_image, errors = docker_base.pull_or_create_test_image( - docker_image, - integration_script.subtype or integration_script.type, - log_prompt=f"Unit test {integration_script.name}", - ) - if errors: - raise RuntimeError(f"Creating docker failed due to {errors}") - (integration_script.path.parent / "conftest.py").unlink(missing_ok=True) - (integration_script.path.parent / "conftest.py").symlink_to( - ( - CONTENT_PATH - / "Tests" - / "scripts" - / "dev_envs" - / "pytest" - / "conftest.py" - ) - ) - - logger.info( - f"Running test for {relative_integration_script_path} using {docker_image=} with {test_docker_image=}" - ) - container = docker_client.containers.run( - image=test_docker_image, - environment={ - "PYTHONPATH": ":".join(DOCKER_PYTHONPATH), - "REQUESTS_CA_BUNDLE": "/etc/ssl/certs/ca-certificates.crt", - "PYTHONDONTWRITEBYTECODE": "1", - }, - volumes=[ - f"{CONTENT_PATH}:/content", - ], - command=PWSH_COMMAND - if integration_script.type == "powershell" - else [PYTEST_COMMAND], - user=f"{os.getuid()}:{os.getgid()}", - working_dir=working_dir, - detach=True, - ) - logger.debug(f"Running test in container {container.id}") - stream_docker_container_output( - container.logs(stream=True), - logger.info if verbose else logger.debug, - ) - # wait for container to finish - if status_code := container.wait()["StatusCode"]: - if status_code == NO_TESTS_COLLECTED: - logger.warning( - f"No test are collected for {relative_integration_script_path} using {docker_image}." - ) - continue - if not ( - integration_script.path.parent / ".report_pytest.xml" - ).exists(): - raise Exception( - f"No pytest report found in {relative_integration_script_path.parent}. Logs: {container.logs()}" - ) - test_failed = False - for suite in JUnitXml.fromfile( - integration_script.path.parent / ".report_pytest.xml" - ): - for case in suite: - if not case.is_passed: - logger.error( - f"Test for {integration_script.object_id} failed in {case.name} with error {case.result[0].message}: {case.result[0].text}" - ) - test_failed = True - if not test_failed: - logger.error( - f"Error running unit tests for {relative_integration_script_path} using {docker_image=}. Container reports {status_code=}, logs: {container.logs()}" - ) - exit_code = 1 - else: - logger.info( - f"[green]All tests passed for {relative_integration_script_path} in {docker_image}[/green]" - ) - container.remove(force=True) - except Exception as e: - logger.error( - f"Failed to run test for {filename} in {docker_image}: {e}" - ) - traceback.print_exc() - exit_code = 1 - return exit_code diff --git a/demisto_sdk/commands/setup_env/setup_environment.py b/demisto_sdk/commands/setup_env/setup_environment.py index e3a72e5d5e..d7900f00e9 100644 --- a/demisto_sdk/commands/setup_env/setup_environment.py +++ b/demisto_sdk/commands/setup_env/setup_environment.py @@ -476,7 +476,7 @@ def configure_integration( "Failed to initialize docker client. Make sure Docker is running and configured correctly" ) raise - (test_docker_image, errors,) = docker_helper.get_docker().pull_or_create_test_image( + (test_docker_image, errors,) = docker_helper.get_docker().get_or_create_test_image( docker_image, integration_script.type ) if errors: diff --git a/demisto_sdk/commands/setup_env/tests/setup_environment_test.py b/demisto_sdk/commands/setup_env/tests/setup_environment_test.py index 640f19ac3d..048a39259e 100644 --- a/demisto_sdk/commands/setup_env/tests/setup_environment_test.py +++ b/demisto_sdk/commands/setup_env/tests/setup_environment_test.py @@ -45,7 +45,7 @@ def test_setup_env_vscode(mocker, monkeypatch, pack, create_virtualenv): mocker.patch.object(content_item, "CONTENT_PATH", repo_path) mocker.patch.object( docker_helper.DockerBase, - "pull_or_create_test_image", + "get_or_create_test_image", return_value=(test_image, ""), ) mocker.patch.object( diff --git a/demisto_sdk/scripts/merge_pytest_reports.py b/demisto_sdk/scripts/merge_pytest_reports.py index 900e2acad8..56825fe130 100644 --- a/demisto_sdk/scripts/merge_pytest_reports.py +++ b/demisto_sdk/scripts/merge_pytest_reports.py @@ -35,10 +35,10 @@ def fix_coverage_report_path(coverage_file: Path) -> bool: 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"): + if not file.startswith("/src"): # means that the .coverage file is already fixed continue - file = Path(file).relative_to("/content") + file = Path(file).relative_to("/src") if ( not (CONTENT_PATH / file).exists() or file.parent.name @@ -59,8 +59,8 @@ def fix_coverage_report_path(coverage_file: Path) -> bool: 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) + logger.warning(f"Broken .coverage file found: {coverage_file}, deleting it") + coverage_file.unlink(missing_ok=True) return False @@ -80,7 +80,9 @@ def merge_coverage_report(): def merge_junit_reports(): - report_files = CONTENT_PATH.rglob(".report_pytest.xml") + junit_reports_path = CONTENT_PATH / ".pre-commit" / "pytest-junit" + + report_files = junit_reports_path.iterdir() if reports := [JUnitXml.fromfile(str(file)) for file in report_files]: report = reports[0] for rep in reports[1:]: diff --git a/poetry.lock b/poetry.lock index 1b4c8ef6dd..97ce515240 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2961,13 +2961,13 @@ testing = ["pytest", "pytest-benchmark"] [[package]] name = "pre-commit" -version = "2.21.0" +version = "3.5.0" description = "A framework for managing and maintaining multi-language pre-commit hooks." optional = false -python-versions = ">=3.7" +python-versions = ">=3.8" files = [ - {file = "pre_commit-2.21.0-py2.py3-none-any.whl", hash = "sha256:e2f91727039fc39a92f58a588a25b87f936de6567eed4f0e673e0507edc75bad"}, - {file = "pre_commit-2.21.0.tar.gz", hash = "sha256:31ef31af7e474a8d8995027fefdfcf509b5c913ff31f2015b4ec4beb26a6f658"}, + {file = "pre_commit-3.5.0-py2.py3-none-any.whl", hash = "sha256:841dc9aef25daba9a0238cd27984041fa0467b4199fc4852e27950664919f660"}, + {file = "pre_commit-3.5.0.tar.gz", hash = "sha256:5804465c675b659b0862f07907f96295d490822a450c4c40e747d0b1c6ebcb32"}, ] [package.dependencies] @@ -5087,4 +5087,4 @@ generate-unit-tests = ["klara"] [metadata] lock-version = "2.0" python-versions = ">=3.8,<3.11" -content-hash = "257fe8b92749c8d1499ebffc0d5b8a87b5c00728d60219e90c66202b3a0561f8" +content-hash = "dc3f26c54e8c6b1c96ef54d8ee5850731574f66219c6325ce821efd23bc9c9d2" diff --git a/pyproject.toml b/pyproject.toml index fb9633722a..a9adfb9ad9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,11 +111,11 @@ klara = { version = "^0.6.3", optional = true } +pre-commit = "^3.5.0" [tool.poetry.group.dev.dependencies] mitmproxy = "^8.0.0" mock = "^4.0.3" -pre-commit = "^2.18.1" pytest = "^7.1.1" pytest-cov = "^4.0.0" pytest-datadir-ng = "^1.1.1"