From 429e986ddfd4bc507eb98cf6b3660ff36fd696b1 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 18 Jul 2019 11:01:21 -0700 Subject: [PATCH 1/4] Install pack with the latest tag First part of Fixes StackStorm/discussions#354. If pack version is not specified, install the latest version from pack index.json (https://index.stackstorm.org/v1/index.json), otherwise install from master branch as today. Note: Direct github/repo install is not considered an Exchange index hit, therefore it is not covert by this fix. --- CHANGELOG.rst | 6 ++++++ contrib/packs/tests/test_action_download.py | 8 ++++---- st2common/st2common/util/pack_management.py | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b255f90db5..55a1d244a3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,12 @@ Changelog in development -------------- +Changed +~~~~~~~ + +* Install pack with the latest tag version if it exists when branch is not specialized. + (improvement) #4743 + Fixed ~~~~~ diff --git a/contrib/packs/tests/test_action_download.py b/contrib/packs/tests/test_action_download.py index dc31c492e7..cab0694100 100644 --- a/contrib/packs/tests/test_action_download.py +++ b/contrib/packs/tests/test_action_download.py @@ -505,6 +505,9 @@ def side_effect(ref): ] for default_branch, ref in edge_cases: + if not ref: + ref = PACK_INDEX['test']['version'] + self.repo_instance.git = mock.MagicMock( branch=(lambda *args: default_branch), checkout=(lambda *args: True) @@ -529,10 +532,7 @@ def fake_commit(arg_ref): action = self.get_action_instance() - if ref: - packs = ['test=%s' % (ref)] - else: - packs = ['test'] + packs = ['test=%s' % (ref)] result = action.run(packs=packs, abs_repo_base=self.repo_base) self.assertEqual(result, {'test': 'Success.'}) diff --git a/st2common/st2common/util/pack_management.py b/st2common/st2common/util/pack_management.py index a84b41553d..ab323ffcb6 100644 --- a/st2common/st2common/util/pack_management.py +++ b/st2common/st2common/util/pack_management.py @@ -370,7 +370,7 @@ def get_repo_url(pack, proxy_config=None): if not pack: raise Exception('No record of the "%s" pack in the index.' % (name_or_url)) - return (pack['repo_url'], version) + return (pack['repo_url'], version or pack['version']) else: return (eval_repo_url(name_or_url), version) From 3b0a7566b4fdd594ffb67446c462b1b54488e04e Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Fri, 19 Jul 2019 13:25:43 -0700 Subject: [PATCH 2/4] Revert changes from last commit and fix failed testcase Root cause: Value passed to arg_ref for `fake_commit` is `v0.4.0` after new code changes, but local `ref` is None. So it failed comparison at line https://github.com/StackStorm/st2/blob/c0458f1a06c2a81cac81697817c422ffb7631047/contrib/packs/tests/test_action_download.py#L523 and mocked `gitref` value is not returned. --- contrib/packs/tests/test_action_download.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/packs/tests/test_action_download.py b/contrib/packs/tests/test_action_download.py index cab0694100..fc903f3373 100644 --- a/contrib/packs/tests/test_action_download.py +++ b/contrib/packs/tests/test_action_download.py @@ -505,9 +505,6 @@ def side_effect(ref): ] for default_branch, ref in edge_cases: - if not ref: - ref = PACK_INDEX['test']['version'] - self.repo_instance.git = mock.MagicMock( branch=(lambda *args: default_branch), checkout=(lambda *args: True) @@ -523,7 +520,7 @@ def side_effect(ref): # Fool _get_gitref into working when its ref == our ref def fake_commit(arg_ref): - if arg_ref == ref: + if not ref or arg_ref == ref: return gitref else: raise BadName() @@ -532,7 +529,10 @@ def fake_commit(arg_ref): action = self.get_action_instance() - packs = ['test=%s' % (ref)] + if ref: + packs = ['test=%s' % (ref)] + else: + packs = ['test'] result = action.run(packs=packs, abs_repo_base=self.repo_base) self.assertEqual(result, {'test': 'Success.'}) From 219c9a263feaa87e9696a5a9a2ccde3b3e6423ad Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Mon, 22 Jul 2019 17:41:55 -0700 Subject: [PATCH 3/4] New dedicated test case for checkout latest tag --- contrib/packs/tests/test_action_download.py | 36 +++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/contrib/packs/tests/test_action_download.py b/contrib/packs/tests/test_action_download.py index fc903f3373..bb3170a9f3 100644 --- a/contrib/packs/tests/test_action_download.py +++ b/contrib/packs/tests/test_action_download.py @@ -81,6 +81,17 @@ def mock_is_dir_func(path): return original_is_dir_func(path) +def mock_get_gitref(repo, ref): + """ + Mock get_gitref function which return mocked object if ref passed is + PACK_INDEX['test']['version'] + """ + if ref == 'v%s' % PACK_INDEX['test']['version']: + return mock.MagicMock(hexsha=PACK_INDEX['test']['version']) + else: + return None + + @mock.patch.object(pack_service, 'fetch_pack_index', mock.MagicMock(return_value=(PACK_INDEX, {}))) class DownloadGitRepoActionTestCase(BaseActionTestCase): action_cls = DownloadGitRepoAction @@ -572,3 +583,28 @@ def test_run_pack_download_local_directory(self): destination_path = os.path.join(self.repo_base, 'test4') self.assertTrue(os.path.exists(destination_path)) self.assertTrue(os.path.exists(os.path.join(destination_path, 'pack.yaml'))) + + @mock.patch('st2common.util.pack_management.get_gitref', mock_get_gitref) + def test_run_pack_download_with_tag(self): + action = self.get_action_instance() + result = action.run(packs=['test'], abs_repo_base=self.repo_base) + temp_dir = hashlib.md5(PACK_INDEX['test']['repo_url'].encode()).hexdigest() + + self.assertEqual(result, {'test': 'Success.'}) + self.clone_from.assert_called_once_with(PACK_INDEX['test']['repo_url'], + os.path.join(os.path.expanduser('~'), temp_dir)) + self.assertTrue(os.path.isfile(os.path.join(self.repo_base, 'test/pack.yaml'))) + + # Check repo.git.checkout is called three times + self.assertEqual(self.repo_instance.git.checkout.call_count, 3) + + # Check repo.git.checkout called with latest tag or branch + self.assertEqual(PACK_INDEX['test']['version'], + self.repo_instance.git.checkout.call_args_list[1][0][0]) + + # Check repo.git.checkout called with head + self.assertEqual(self.repo_instance.head.reference, + self.repo_instance.git.checkout.call_args_list[2][0][0]) + + self.repo_instance.git.branch.assert_called_with( + '-f', self.repo_instance.head.reference, PACK_INDEX['test']['version']) From 8547e1c6aee201c95dd44d49ec48a762be30432d Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Tue, 23 Jul 2019 11:21:30 -0700 Subject: [PATCH 4/4] Better way to handle mock_get_gitref return value --- contrib/packs/tests/test_action_download.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/contrib/packs/tests/test_action_download.py b/contrib/packs/tests/test_action_download.py index bb3170a9f3..7d0becc2b3 100644 --- a/contrib/packs/tests/test_action_download.py +++ b/contrib/packs/tests/test_action_download.py @@ -86,8 +86,13 @@ def mock_get_gitref(repo, ref): Mock get_gitref function which return mocked object if ref passed is PACK_INDEX['test']['version'] """ - if ref == 'v%s' % PACK_INDEX['test']['version']: - return mock.MagicMock(hexsha=PACK_INDEX['test']['version']) + if PACK_INDEX['test']['version'] in ref: + if ref[0] == 'v': + return mock.MagicMock(hexsha=PACK_INDEX['test']['version']) + else: + return None + elif ref: + return mock.MagicMock(hexsha="abcDef") else: return None