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
demisto · Nov 16, 2023 · e0c602d · e0c602d
1 parent 46033f4
commit e0c602d
Show file tree

Hide file tree

Showing 17 changed files with 262 additions and 187 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)