Merge pull request #325 from demisto/validate_against_release_branch

Validate against release branch

demisto_sdk/commands/validate/file_validator.py

@@ -54,7 +54,8 @@
 from demisto_sdk.commands.common.tools import (LOG_COLORS, checked_type,
                                                filter_packagify_changes,
                                                find_type, get_pack_name,
-                                               get_yaml, get_yml_paths_in_dir,
+                                               get_remote_file, get_yaml,
+                                               get_yml_paths_in_dir,
                                                is_file_path_in_pack,
                                                print_color, print_error,
                                                print_warning, run_command)
@@ -77,7 +78,7 @@ class FilesValidator:
         configuration (Configuration): Configurations for IDSetValidator.
     """
 
-    def __init__(self, is_backward_check=True, prev_ver='origin/master', use_git=False, is_circle=False,
+    def __init__(self, is_backward_check=True, prev_ver=None, use_git=False, is_circle=False,
                  print_ignored_files=False, validate_conf_json=True, validate_id_set=False, file_path=None,
                  configuration=Configuration()):
         self.branch_name = ''
@@ -88,10 +89,6 @@ def __init__(self, is_backward_check=True, prev_ver='origin/master', use_git=Fal
             print(f'Running validation on branch {self.branch_name}')
 
         self.prev_ver = prev_ver
-        if not self.prev_ver:
-            # validate against master if no version was provided
-            self.prev_ver = 'origin/master'
-
         self._is_valid = True
         self.configuration = configuration
         self.is_backward_check = is_backward_check
@@ -601,7 +598,6 @@ def is_valid_structure(self):
                                                  not self.branch_name.startswith('20.')):
                 print('Validates only committed files')
                 self.validate_committed_files()
-                self.validate_against_previous_version(no_error=True)
             else:
                 self.validate_against_previous_version(no_error=True)
                 print('Validates all of Content repo directories according to their schemas')
@@ -623,17 +619,24 @@ def validate_against_previous_version(self, no_error=False):
         Args:
             no_error (bool): If set to true will restore self._is_valid after run (will not return new errors)
         """
-        if self.prev_ver and self.prev_ver != 'master':
-            print_color('Starting validation against {}'.format(self.prev_ver), LOG_COLORS.GREEN)
-            modified_files, _, _, _ = self.get_modified_and_added_files(self.prev_ver)
-            prev_self_valid = self._is_valid
-            self.validate_modified_files(modified_files)
-            if no_error:
-                self._is_valid = prev_self_valid
+        if not self.prev_ver:
+            content_release_branch_id = self.get_content_release_identifier()
+            if not content_release_branch_id:
+                print_warning('could\'t get content\'s release branch ID. Skipping validation.')
+                return
+            else:
+                self.prev_ver = content_release_branch_id
+
+        print_color('Starting validation against {}'.format(self.prev_ver), LOG_COLORS.GREEN)
+        modified_files, _, _, _ = self.get_modified_and_added_files(self.prev_ver)
+        prev_self_valid = self._is_valid
+        self.validate_modified_files(modified_files)
+        if no_error:
+            self._is_valid = prev_self_valid
 
     # parser.add_argument('-t', '--test-filter', type=str2bool, default=False,
     #                     help='Check that tests are valid.')
-    # TODO: after validation there was a step to run the configure_tests script to check that each changed file
+    # TODO: after validation there was a step to run the configure_tests script to check each changed file
     #  had a relevant test - was used as part of the hooks.
 
     @staticmethod
@@ -651,3 +654,11 @@ def _is_py_script_or_integration(file_path):
             return True
 
         return False
+
+    def get_content_release_identifier(self):
+        try:
+            file_content = get_remote_file('.circleci/config.yml', tag=self.branch_name)
+        except Exception:
+            return
+        else:
+            return file_content.get('jobs').get('build').get('environment').get('GIT_SHA1')

demisto_sdk/commands/validate/tests/validators_test.py

@@ -1,3 +1,4 @@
+import json
 import os
 from shutil import copyfile
 from typing import Any, Type
@@ -27,8 +28,9 @@
 from demisto_sdk.commands.unify.unifier import Unifier
 from demisto_sdk.commands.validate.file_validator import FilesValidator
 from demisto_sdk.tests.constants_test import (
-    BETA_INTEGRATION_TARGET, DASHBOARD_TARGET, INCIDENT_FIELD_TARGET,
-    INCIDENT_TYPE_TARGET, INTEGRATION_RELEASE_NOTES_TARGET, INTEGRATION_TARGET,
+    BETA_INTEGRATION_TARGET, DASHBOARD_TARGET, GIT_HAVE_MODIFIED_AND_NEW_FILES,
+    INCIDENT_FIELD_TARGET, INCIDENT_TYPE_TARGET,
+    INTEGRATION_RELEASE_NOTES_TARGET, INTEGRATION_TARGET,
     INVALID_DASHBOARD_PATH, INVALID_INCIDENT_FIELD_PATH,
     INVALID_INTEGRATION_ID_PATH, INVALID_LAYOUT_PATH,
     INVALID_MULTI_LINE_1_CHANGELOG_PATH, INVALID_MULTI_LINE_2_CHANGELOG_PATH,
@@ -264,6 +266,28 @@ def test_is_all_params_not_hidden(self, source, answer):
         validator = IntegrationValidator(structure)
         assert validator.is_all_params_not_hidden() is answer
 
+    with open(GIT_HAVE_MODIFIED_AND_NEW_FILES, "r") as test_params_file:
+        tests_params = json.load(test_params_file)
+    params = [
+        (None, tuple(set(i) for i in tests_params['data']['params_with_data']), '123456', True, True),
+        ('origin/master', tuple(set(i) for i in tests_params['data']['params_with_data']), '123456', True, True),
+        (None, tuple(set(i) for i in tests_params['data']['params_with_data']), '', True, True),
+        (None, tuple(set(i) for i in tests_params['data']['params_without_data']), '123456', True, True),
+        (None, tuple(set(i) for i in tests_params['data']['params_with_data']), '123456', False, False),
+    ]
+
+    @pytest.mark.parametrize("prev_var, get_modified_and_added_files, release_iden, answer, is_valid", params)
+    def test_validate_against_previous_version(self, prev_var, get_modified_and_added_files, release_iden, answer,
+                                               is_valid, mocker):
+        file_validator = FilesValidator(validate_conf_json=False, prev_ver=prev_var)
+        file_validator._is_valid = is_valid
+        mocker.patch.object(FilesValidator, 'get_modified_and_added_files', return_value=get_modified_and_added_files)
+        mocker.patch.object(FilesValidator, 'get_content_release_identifier', return_value=release_iden)
+        mocker.patch.object(FilesValidator, 'validate_modified_files', return_value=None)
+
+        assert file_validator.validate_against_previous_version() is None
+        assert file_validator._is_valid is answer
+
     INPUTS_STRUCTURE_VALIDATION = [
         (VALID_INTEGRATION_TEST_PATH, INTEGRATION_TARGET),
         (VALID_SCRIPT_PATH, SCRIPT_TARGET),

demisto_sdk/tests/constants_test.py

@@ -75,8 +75,7 @@
 EQUAL_VAL_PATH = f'Playbooks'
 INVALID_NO_HIDDEN_PARAMS = f"{GIT_ROOT}/demisto_sdk/tests/test_files/invalid-no-hidden-params.yml"
 VALID_NO_HIDDEN_PARAMS = f"{GIT_ROOT}/demisto_sdk/tests/test_files/valid-no-hidden-params.yml"
-
-
+GIT_HAVE_MODIFIED_AND_NEW_FILES = f"{GIT_ROOT}/demisto_sdk/tests/test_files/git_have_modified_and_new_files.json"
 SOURCE_FORMAT_INCIDENTFIELD_COPY = f"{GIT_ROOT}/demisto_sdk/tests/test_files/format_incidentfield-copy.json"
 DESTINATION_FORMAT_INCIDENTFIELD_COPY = f"IncidentFields/incidentfield-copy.json"
 INCIDENTFIELD_PATH = f"IncidentFields"

demisto_sdk/tests/test_files/git_have_modified_and_new_files.json

@@ -0,0 +1,18 @@
+{
+    "data" : {
+        "params_without_data" : {
+            "modified_files1" : [""],
+            "added_files1" : [""],
+            "old_format_files1" : [""],
+            "packs1" : [""]
+    },
+
+        "params_with_data" : [
+            ["/path/mod_file1", "/path/mod_file2", "/path/mod_file3"],
+            ["/path/add_file1", "/path/add_file2", "/path/add_file3"],
+            ["/path/old_file1", "/path/old_file2", "/path/old_file3"],
+            ["/path/pack_file1", "/path/pack_file2", "/path/pack_file3"]
+
+        ]
+    }
+}