From 2af2227fbab05273b351605c3e821a5f75a0a168 Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Tue, 20 Jun 2023 18:01:03 +0300 Subject: [PATCH 1/9] [marketplace contributions] - fix issue where support labels are not added --- Utils/github_workflow_scripts/handle_external_pr.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Utils/github_workflow_scripts/handle_external_pr.py b/Utils/github_workflow_scripts/handle_external_pr.py index ec82be2b789b..99f5813bc41c 100755 --- a/Utils/github_workflow_scripts/handle_external_pr.py +++ b/Utils/github_workflow_scripts/handle_external_pr.py @@ -124,10 +124,12 @@ def get_packs_support_level_label(file_paths: List[str], external_pr_branch: str f'to retrieve support level of {pack_dirs_to_check_support_levels_labels}' ) try: + fork_owner = os.getenv('GITHUB_ACTOR') with Checkout( repo=Repo(Path().cwd(), search_parent_directories=True), branch_to_checkout=external_pr_branch, - fork_owner=os.getenv('GITHUB_ACTOR') + # marketplace contributions the name of the forked owner is not the same as the name of the owner + fork_owner=fork_owner if fork_owner != 'xsoar-bot' else 'xsoar-contrib' ): packs_support_levels = get_packs_support_levels(pack_dirs_to_check_support_levels_labels) except Exception as error: From 355b66bee30a246ae5738835c981c073e05b024c Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Wed, 21 Jun 2023 15:25:24 +0300 Subject: [PATCH 2/9] add unit-tests --- .../handle_external_pr_test.py | 35 +++++++++++++++++++ .../test_files/Packs/Pack1/pack_metadata.json | 3 ++ Utils/github_workflow_scripts/utils.py | 17 +++++++++ 3 files changed, 55 insertions(+) create mode 100644 Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files/Packs/Pack1/pack_metadata.json diff --git a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py index 98779eca2963..f09e12b56c23 100644 --- a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py +++ b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py @@ -1,3 +1,4 @@ +import os import pytest @@ -38,3 +39,37 @@ def test_get_highest_support_label(support_levels, expected_support_level): """ from Utils.github_workflow_scripts.handle_external_pr import get_highest_support_label assert get_highest_support_label(support_levels) == expected_support_level + + +@pytest.mark.parametrize( + 'fork_owner, expected_fork_owner', [ + ('test', 'test'), + ('xsoar-bot', 'xsoar-contrib') + ] +) +def test_get_packs_support_level_label(mocker, fork_owner, expected_fork_owner): + """ + Given: + a pack and a fork owner + + When: + running get_packs_support_level_label function + + Then: + - make sure correct support label is returned and the + - fork owner that is being delivered to the Checkout branch is correct. + """ + from Utils.github_workflow_scripts.handle_external_pr import get_packs_support_level_label, Checkout + from Utils.github_workflow_scripts.utils import ChangeCWD + + checkout_mocker = mocker.patch.object(Checkout, '__init__', return_value=None) + mocker.patch.object(Checkout, '__enter__', return_value=None) + mocker.patch.object(Checkout, '__exit__', return_value=None) + mocker.patch.object(os, 'getenv', return_value=fork_owner) + + with ChangeCWD('Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files'): + assert get_packs_support_level_label( + file_paths=['test_files/Packs/Pack1/pack_metadata.json'], external_pr_branch='test' + ) == 'Xsoar Support Level' + + assert checkout_mocker.call_args.kwargs['fork_owner'] == expected_fork_owner diff --git a/Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files/Packs/Pack1/pack_metadata.json b/Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files/Packs/Pack1/pack_metadata.json new file mode 100644 index 000000000000..bc7b6f206c6b --- /dev/null +++ b/Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files/Packs/Pack1/pack_metadata.json @@ -0,0 +1,3 @@ +{ + "support": "xsoar" +} \ No newline at end of file diff --git a/Utils/github_workflow_scripts/utils.py b/Utils/github_workflow_scripts/utils.py index 52dd6cfa912a..837652654cc9 100644 --- a/Utils/github_workflow_scripts/utils.py +++ b/Utils/github_workflow_scripts/utils.py @@ -145,3 +145,20 @@ def __exit__(self, *args): """Checks out the previous branch""" self.repo.git.checkout(self._original_branch) print(f"Checked out to original branch {self._original_branch}") + + +class ChangeCWD: + """ + Temporary changes the cwd to the given dir and then reverts it. + Use with 'with' statement. + """ + + def __init__(self, directory): + self.current = os.getcwd() + self.directory = directory + + def __enter__(self): + os.chdir(self.directory) + + def __exit__(self, *args): + os.chdir(self.current) From 5173fad30965c277054a9a655635d7b760d4383b Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Wed, 21 Jun 2023 16:30:57 +0300 Subject: [PATCH 3/9] update comment --- Utils/github_workflow_scripts/handle_external_pr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Utils/github_workflow_scripts/handle_external_pr.py b/Utils/github_workflow_scripts/handle_external_pr.py index 99f5813bc41c..66b17541cbc6 100755 --- a/Utils/github_workflow_scripts/handle_external_pr.py +++ b/Utils/github_workflow_scripts/handle_external_pr.py @@ -128,7 +128,7 @@ def get_packs_support_level_label(file_paths: List[str], external_pr_branch: str with Checkout( repo=Repo(Path().cwd(), search_parent_directories=True), branch_to_checkout=external_pr_branch, - # marketplace contributions the name of the forked owner is not the same as the name of the owner + # in marketplace contributions the name of the owner should be xsoar-contrib fork_owner=fork_owner if fork_owner != 'xsoar-bot' else 'xsoar-contrib' ): packs_support_levels = get_packs_support_levels(pack_dirs_to_check_support_levels_labels) From 9454f0ad74868152bb173f449ab5696344097a65 Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Wed, 21 Jun 2023 16:32:09 +0300 Subject: [PATCH 4/9] update path of test --- .../github_workflow_scripts_tests/handle_external_pr_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py index f09e12b56c23..62334e336c31 100644 --- a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py +++ b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py @@ -69,7 +69,7 @@ def test_get_packs_support_level_label(mocker, fork_owner, expected_fork_owner): with ChangeCWD('Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files'): assert get_packs_support_level_label( - file_paths=['test_files/Packs/Pack1/pack_metadata.json'], external_pr_branch='test' + file_paths=['Packs/Pack1/pack_metadata.json'], external_pr_branch='test' ) == 'Xsoar Support Level' assert checkout_mocker.call_args.kwargs['fork_owner'] == expected_fork_owner From 8f7fac2082b965434e7a0cc3b4cd454642eb15a9 Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Wed, 21 Jun 2023 16:36:00 +0300 Subject: [PATCH 5/9] path cwd --- Utils/github_workflow_scripts/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Utils/github_workflow_scripts/utils.py b/Utils/github_workflow_scripts/utils.py index 837652654cc9..6565e5c22d59 100644 --- a/Utils/github_workflow_scripts/utils.py +++ b/Utils/github_workflow_scripts/utils.py @@ -4,6 +4,7 @@ import json from datetime import datetime from typing import Any, Generator, Iterable, Optional, Tuple, Union +from pathlib import Path from git import Repo @@ -154,7 +155,7 @@ class ChangeCWD: """ def __init__(self, directory): - self.current = os.getcwd() + self.current = Path().cwd() self.directory = directory def __enter__(self): From 377f7054a26df62cf359b94735d4341599ecf9e3 Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Thu, 22 Jun 2023 09:05:00 +0300 Subject: [PATCH 6/9] fallback to master in case checkout failed --- .../handle_external_pr_test.py | 24 ++++++++++++++++++- .../handle_external_pr.py | 7 ++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py index 62334e336c31..3ac66a50c807 100644 --- a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py +++ b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py @@ -56,7 +56,7 @@ def test_get_packs_support_level_label(mocker, fork_owner, expected_fork_owner): running get_packs_support_level_label function Then: - - make sure correct support label is returned and the + - make sure correct support label is returned. - fork owner that is being delivered to the Checkout branch is correct. """ from Utils.github_workflow_scripts.handle_external_pr import get_packs_support_level_label, Checkout @@ -73,3 +73,25 @@ def test_get_packs_support_level_label(mocker, fork_owner, expected_fork_owner): ) == 'Xsoar Support Level' assert checkout_mocker.call_args.kwargs['fork_owner'] == expected_fork_owner + + +def test_get_packs_support_level_label_checkout_failed(mocker): + """ + Given: + a pack + + When: + running get_packs_support_level_label function when Checkout fails. + + Then: + - make sure correct support label is still returned. + """ + from Utils.github_workflow_scripts.handle_external_pr import get_packs_support_level_label, Checkout + from Utils.github_workflow_scripts.utils import ChangeCWD + + mocker.patch.object(Checkout, '__init__', return_value=Exception('Error')) + + with ChangeCWD('Utils/github_workflow_scripts/github_workflow_scripts_tests/test_files'): + assert get_packs_support_level_label( + file_paths=['Packs/Pack1/pack_metadata.json'], external_pr_branch='test' + ) == 'Xsoar Support Level' diff --git a/Utils/github_workflow_scripts/handle_external_pr.py b/Utils/github_workflow_scripts/handle_external_pr.py index 66b17541cbc6..32658c47baff 100755 --- a/Utils/github_workflow_scripts/handle_external_pr.py +++ b/Utils/github_workflow_scripts/handle_external_pr.py @@ -133,8 +133,11 @@ def get_packs_support_level_label(file_paths: List[str], external_pr_branch: str ): packs_support_levels = get_packs_support_levels(pack_dirs_to_check_support_levels_labels) except Exception as error: - packs_support_levels = set() - print(f'Received error when trying to checkout to {external_pr_branch} forked content repo\n{error=}') + # in case we were not able to checkout correctly, fallback to the files in the master branch to retrieve support labels + # in case those files exist. + print(f'Received error when trying to checkout to {external_pr_branch} \n{error=}') + print('Trying to retrieve support levels from the base branch') + packs_support_levels = get_packs_support_levels(pack_dirs_to_check_support_levels_labels) print(f'{packs_support_levels=}') return get_highest_support_label(packs_support_levels) if packs_support_levels else '' From b7a5bfe2e5889afe81de1d4d15c4cd0d856477fc Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Thu, 22 Jun 2023 09:06:19 +0300 Subject: [PATCH 7/9] docstrings improvments --- .../handle_external_pr_test.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py index 3ac66a50c807..a5f11b792609 100644 --- a/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py +++ b/Utils/github_workflow_scripts/github_workflow_scripts_tests/handle_external_pr_test.py @@ -29,13 +29,13 @@ def test_get_highest_support_label(support_levels, expected_support_level): """ Given: - a list of support levels for packs that were changed + - a list of support levels for packs that were changed When: - running get_highest_support_label function + - running get_highest_support_label function Then: - make sure the highest support level is always returned + - make sure the highest support level is always returned """ from Utils.github_workflow_scripts.handle_external_pr import get_highest_support_label assert get_highest_support_label(support_levels) == expected_support_level @@ -50,10 +50,10 @@ def test_get_highest_support_label(support_levels, expected_support_level): def test_get_packs_support_level_label(mocker, fork_owner, expected_fork_owner): """ Given: - a pack and a fork owner + - a pack and a fork owner When: - running get_packs_support_level_label function + - running get_packs_support_level_label function Then: - make sure correct support label is returned. @@ -78,10 +78,10 @@ def test_get_packs_support_level_label(mocker, fork_owner, expected_fork_owner): def test_get_packs_support_level_label_checkout_failed(mocker): """ Given: - a pack + - a pack When: - running get_packs_support_level_label function when Checkout fails. + - running get_packs_support_level_label function when Checkout fails. Then: - make sure correct support label is still returned. From ad32c3f752aa7892e6a0a6d63d6daea7efe996ba Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Thu, 22 Jun 2023 09:08:46 +0300 Subject: [PATCH 8/9] update print string --- Utils/github_workflow_scripts/handle_external_pr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Utils/github_workflow_scripts/handle_external_pr.py b/Utils/github_workflow_scripts/handle_external_pr.py index 32658c47baff..c9e551b90478 100755 --- a/Utils/github_workflow_scripts/handle_external_pr.py +++ b/Utils/github_workflow_scripts/handle_external_pr.py @@ -136,7 +136,7 @@ def get_packs_support_level_label(file_paths: List[str], external_pr_branch: str # in case we were not able to checkout correctly, fallback to the files in the master branch to retrieve support labels # in case those files exist. print(f'Received error when trying to checkout to {external_pr_branch} \n{error=}') - print('Trying to retrieve support levels from the base branch') + print('Trying to retrieve support levels from the master branch') packs_support_levels = get_packs_support_levels(pack_dirs_to_check_support_levels_labels) print(f'{packs_support_levels=}') From 0c5473733fdbe45876132022c8e5abefc977a1af Mon Sep 17 00:00:00 2001 From: GuyAfik Date: Thu, 22 Jun 2023 13:45:34 +0300 Subject: [PATCH 9/9] add prints --- Utils/github_workflow_scripts/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Utils/github_workflow_scripts/utils.py b/Utils/github_workflow_scripts/utils.py index 6565e5c22d59..aabe22e5e010 100644 --- a/Utils/github_workflow_scripts/utils.py +++ b/Utils/github_workflow_scripts/utils.py @@ -109,6 +109,7 @@ def __init__(self, repo: Repo, branch_to_checkout: str, fork_owner: Optional[str url = f"https://github.com/{fork_owner}/{repo_name}" try: self.repo.create_remote(name=forked_remote_name, url=url) + print(f'Successfully created remote {forked_remote_name} for repo {url}') except Exception as error: print(f'could not create remote from {url}, {error=}') # handle the case where the name of the forked repo is not content