From 949025b3a7be98bef154787499eb179c89d8f8b3 Mon Sep 17 00:00:00 2001 From: Johan Lorenzo Date: Fri, 22 Feb 2019 19:49:14 +0100 Subject: [PATCH] Bug 1529990 - Fix Mobile bustage --- scriptworker/constants.py | 7 + scriptworker/cot/verify.py | 20 ++- scriptworker/github.py | 160 ++++++++++++++++++++ scriptworker/task.py | 121 ++++++++------- scriptworker/test/test_cot_verify.py | 25 ++-- scriptworker/test/test_github.py | 211 ++++++++++++++++++++++++++- scriptworker/test/test_task.py | 176 ++++++++++++---------- scriptworker/test/test_utils.py | 13 ++ scriptworker/utils.py | 16 ++ 9 files changed, 595 insertions(+), 154 deletions(-) diff --git a/scriptworker/constants.py b/scriptworker/constants.py index f1e34971..46adf3fa 100644 --- a/scriptworker/constants.py +++ b/scriptworker/constants.py @@ -313,6 +313,10 @@ }), 'mobile': frozendict({ 'project:mobile:android-components:releng:beetmover:bucket:maven-production': 'android-components-repo', + 'project:mobile:android-components:releng:beetmover:bucket:maven-snapshot-production': 'android-components-repo', + + 'project:mobile:fenix:releng:googleplay:product:fenix': 'fenix-repo', + 'project:mobile:fenix:releng:signing:cert:release-signing': 'fenix-repo', 'project:mobile:focus:googleplay:product:focus': 'focus-repo', 'project:mobile:focus:releng:signing:cert:release-signing': 'focus-repo', @@ -409,6 +413,9 @@ ), }), 'mobile': frozendict({ + 'fenix-repo': ( + '/mozilla-mobile/fenix', + ), 'focus-repo': ( '/mozilla-mobile/focus-android', ), diff --git a/scriptworker/cot/verify.py b/scriptworker/cot/verify.py index 0d899c63..b64071bb 100644 --- a/scriptworker/cot/verify.py +++ b/scriptworker/cot/verify.py @@ -121,19 +121,18 @@ def dependent_task_ids(self): """ return [x.task_id for x in self.links] - def is_try_or_pull_request(self): + async def is_try_or_pull_request(self): """Determine if any task in the chain is a try task. Returns: bool: True if a task is a try task. """ - result = is_try_or_pull_request(self.context, self.task) - for link in self.links: - if link.is_try_or_pull_request: - result = True - break - return result + tasks = [asyncio.ensure_future(link.is_try_or_pull_request()) for link in self.links] + tasks.insert(0, asyncio.ensure_future(is_try_or_pull_request(self.context, self.task))) + + conditions = await raise_future_exceptions(tasks) + return any(conditions) def get_link(self, task_id): """Get a ``LinkOfTrust`` by task id. @@ -243,10 +242,9 @@ def task(self, task): self.parent_task_id = get_parent_task_id(self.task) self.worker_impl = guess_worker_impl(self) - @property - def is_try_or_pull_request(self): + async def is_try_or_pull_request(self): """bool: the task is either a try or a pull request one.""" - return is_try_or_pull_request(self.context, self.task) + return await is_try_or_pull_request(self.context, self.task) @property def cot(self): @@ -2072,7 +2070,7 @@ async def trace_back_to_tree(chain): obj.name, obj.task_id )) # Disallow restricted privs on is_try_or_pull_request. This may be a redundant check. - if restricted_privs and chain.is_try_or_pull_request(): + if restricted_privs and await chain.is_try_or_pull_request(): errors.append( "{} {} has restricted privilege scope, and is_try_or_pull_request()!".format( chain.name, chain.task_id diff --git a/scriptworker/github.py b/scriptworker/github.py index a441a6d4..4ec069ce 100644 --- a/scriptworker/github.py +++ b/scriptworker/github.py @@ -1,7 +1,18 @@ """GitHub helper functions.""" +import re + from github3 import GitHub +from scriptworker.exceptions import ConfigError +from scriptworker.utils import ( + get_single_item_from_sequence, + get_parts_of_url_path, + retry_request, +) + +_GIT_FULL_HASH_PATTERN = re.compile(r'^[0-9a-f]{40}$') + class GitHubRepository(): """Wrapper around GitHub API. Used to access public data.""" @@ -65,3 +76,152 @@ def get_release(self, tag_name): """ return self._github_repository.release_from_tag(tag_name).as_dict() + + def get_tag_hash(self, tag_name): + """Fetch the commit hash that was tagged with ``tag_name``. + + Args: + tag_name (str): the name of the tag + + Returns: + str: the commit hash linked by the tag + + """ + tag_object = get_single_item_from_sequence( + sequence=self._github_repository.tags(), + condition=lambda tag: tag.name == tag_name, + no_item_error_message='No tag "{}" exist'.format(tag_name), + too_many_item_error_message='Too many tags "{}" found'.format(tag_name), + ) + + return tag_object.commit.sha + + async def has_commit_landed_on_repository(self, context, revision): + """Tell if a commit was landed on the repository or if it just comes from a pull request. + + Args: + context (scriptworker.context.Context): the scriptworker context. + revision (str): the commit hash or the tag name. + + Returns: + bool: True if the commit is present in one of the branches of the main repository + + """ + # Revision may be a tag name. `branch_commits` doesn't work on tags + if not _is_git_full_hash(revision): + revision = self.get_tag_hash(tag_name=revision) + + repo = self._github_repository.html_url + + url = '/'.join([repo.rstrip('/'), 'branch_commits', revision]) + html_data = await retry_request(context, url) + html_text = html_data.strip() + # https://github.com/{repo_owner}/{repo_name}/branch_commits/{revision} just returns some \n + # when the commit hasn't landed on the origin repo. Otherwise, some HTML data is returned - it + # represents the branches on which the given revision is present. + return html_text != '' + + +def is_github_url(url): + """Tell if a given URL matches a Github one. + + Args: + url (str): The URL to test. It can be None. + + Returns: + bool: False if the URL is not a string or if it doesn't match a Github URL + + """ + if isinstance(url, str): + return url.startswith('https://github.com/') + else: + return False + + +def extract_github_repo_owner_and_name(url): + """Given an URL, return the repo name and who owns it. + + Args: + url (str): The URL to the GitHub repository + + Raises: + ValueError: on url that aren't from github + + Returns: + str, str: the owner of the repository, the repository name + + """ + _check_github_url_is_supported(url) + + parts = get_parts_of_url_path(url) + repo_owner = parts[0] + repo_name = parts[1] + + return repo_owner, _strip_trailing_dot_git(repo_name) + + +def extract_github_repo_and_revision_from_source_url(url): + """Given an URL, return the repo name and who owns it. + + Args: + url (str): The URL to the GitHub repository + + Raises: + ValueError: on url that aren't from github or when the revision cannot be extracted + + Returns: + str, str: the owner of the repository, the repository name + + """ + _check_github_url_is_supported(url) + + parts = get_parts_of_url_path(url) + repo_name = parts[1] + try: + revision = parts[3] + except IndexError: + raise ValueError('Revision cannot be extracted from url: {}'.format(url)) + + end_index = url.index(repo_name) + len(repo_name) + repo_url = url[:end_index] + + return _strip_trailing_dot_git(repo_url), revision + + +def _strip_trailing_dot_git(url): + if url.endswith('.git'): + url = url[:-len('.git')] + return url + + +def is_github_repo_owner_the_official_one(context, repo_owner): + """Given a repo_owner, check if it matches the one configured to be the official one. + + Args: + context (scriptworker.context.Context): the scriptworker context. + repo_owner (str): the repo_owner to verify + + Raises: + scriptworker.exceptions.ConfigError: when no official owner was defined + + Returns: + bool: True when ``repo_owner`` matches the one configured to be the official one + + """ + official_repo_owner = context.config['official_github_repos_owner'] + if not official_repo_owner: + raise ConfigError( + 'This worker does not have a defined owner for official GitHub repositories. ' + 'Given "official_github_repos_owner": {}'.format(official_repo_owner) + ) + + return official_repo_owner == repo_owner + + +def _is_git_full_hash(revision): + return _GIT_FULL_HASH_PATTERN.match(revision) is not None + + +def _check_github_url_is_supported(url): + if not is_github_url(url): + raise ValueError('"{}" is not a supported GitHub URL!'.format(url)) diff --git a/scriptworker/task.py b/scriptworker/task.py index feda0b0a..103caa10 100644 --- a/scriptworker/task.py +++ b/scriptworker/task.py @@ -15,16 +15,23 @@ import os import pprint import re -from urllib.parse import unquote, urlparse import taskcluster import taskcluster.exceptions from scriptworker.constants import get_reversed_statuses from scriptworker.exceptions import ScriptWorkerTaskException, WorkerShutdownDuringTask +from scriptworker.github import ( + GitHubRepository, + extract_github_repo_owner_and_name, + extract_github_repo_and_revision_from_source_url, + is_github_repo_owner_the_official_one, + is_github_url, +) from scriptworker.log import get_log_filehandle, pipe_to_log from scriptworker.task_process import TaskProcess from scriptworker.utils import ( + get_parts_of_url_path, match_url_path_callback, match_url_regex, ) @@ -348,46 +355,8 @@ def get_repo_scope(task, name): return repo_scopes[0] -def extract_github_repo_owner_and_name(repo_url): - """Given an URL, return the repo name and who owns it. - - Args: - repo_url (str): The URL to the GitHub repository - - Raises: - ValueError: on repo_url that aren't from github - - Returns: - str, str: the owner of the repository, the repository name - - """ - if not repo_url.startswith('https://github.com/'): - raise ValueError('{} is not a supported GitHub URL!'.format(repo_url)) - - parts = _get_parts_of_url_path(repo_url) - repo_owner = parts[0] - repo_name = parts[1] - - if repo_name.endswith(".git"): - repo_name = repo_name[:-len(".git")] - - return repo_owner, repo_name - - -def _get_parts_of_url_path(url): - parsed = urlparse(url) - path = unquote(parsed.path).lstrip('/') - parts = path.split('/') - return parts - - def _is_try_url(url): - return 'try' in _get_parts_of_url_path(url)[0] - - -def _does_url_come_from_official_repo(context, url): - official_repo_owner = context.config['official_github_repos_owner'] - return not official_repo_owner or official_repo_owner == _get_parts_of_url_path(url)[0] + return 'try' in get_parts_of_url_path(url)[0] def is_try(task, source_env_prefix): @@ -422,10 +391,10 @@ def is_try(task, source_env_prefix): )) -def is_pull_request(context, task): +async def is_pull_request(context, task): """Determine if a task is a pull-request-like task (restricted privs). - This goes further than checking ``tasks_for``. We may or may not want + This goes further than checking ``tasks_for``. We may or may not want to keep this. This checks for the following things:: @@ -433,24 +402,49 @@ def is_pull_request(context, task): * ``task.extra.env.tasks_for`` == "github-pull-request" * ``task.payload.env.MOBILE_HEAD_REPOSITORY`` doesn't come from an official repo * ``task.metadata.source`` doesn't come from an official repo, either + * The last 2 items are landed on the official repo Args: context (scriptworker.context.Context): the scriptworker context. task (dict): the task definition to check. Returns: - bool: True if it's a pull-request + bool: True if it's a pull-request. False if it either comes from the official repos or if + the origin can't be determined. In fact, valid scriptworker tasks don't expose + ``task.extra.env.tasks_for`` or ``task.payload.env.MOBILE_HEAD_REPOSITORY``, for instance. """ - repo = get_repo(task, context.config['source_env_prefix']) or '' - return any(( - task.get('extra', {}).get('tasks_for') == 'github-pull-request', - not _does_url_come_from_official_repo(context, repo), - not _does_url_come_from_official_repo(context, task['metadata'].get('source', '')), - )) + tasks_for = task.get('extra', {}).get('tasks_for') + repo_url_from_payload = get_repo(task, context.config['source_env_prefix']) + revision_from_payload = get_revision(task, context.config['source_env_prefix']) + + metadata_source_url = task['metadata'].get('source', '') + repo_from_source_url, revision_from_source_url = \ + extract_github_repo_and_revision_from_source_url(metadata_source_url) + + conditions = [tasks_for == 'github-pull-request'] + urls_revisions_and_can_skip = ( + (repo_url_from_payload, revision_from_payload, True), + (repo_from_source_url, revision_from_source_url, False), + ) + for repo_url, revision, can_skip in urls_revisions_and_can_skip: + # XXX In the case of scriptworker tasks, neither the repo nor the revision is defined + if not repo_url and can_skip: + continue + + repo_owner, repo_name = extract_github_repo_owner_and_name(repo_url) + conditions.append(not is_github_repo_owner_the_official_one(context, repo_owner)) + + if not revision and can_skip: + continue + github_repository = GitHubRepository(repo_owner, repo_name, context.config['github_oauth_token']) + conditions.append(not await github_repository.has_commit_landed_on_repository(context, revision)) -def is_try_or_pull_request(context, task): + return any(conditions) + + +async def is_try_or_pull_request(context, task): """Determine if a task is a try or a pull-request-like task (restricted privs). Checks are the ones done in ``is_try`` and ``is_pull_request`` @@ -462,10 +456,33 @@ def is_try_or_pull_request(context, task): Returns: bool: True if it's a pull-request or a try task + """ + if is_github_task(task): + return await is_pull_request(context, task) + else: + return is_try(task, context.config['source_env_prefix']) + + +def is_github_task(task): + """Determine if a task is related to GitHub. + + This function currently looks into the ``schedulerId``, ``extra.tasks_for``, and + ``metadata.source``. + + Args: + task (dict): the task definition to check. + + Returns: + bool: True if a piece of data refers to GitHub + """ return any(( - is_try(task, context.config['source_env_prefix']), - is_pull_request(context, task), + # XXX Cron tasks don't usually define 'taskcluster-github' as their schedulerId as they + # are scheduled within another Taskcluster task. + task.get('schedulerId') == 'taskcluster-github', + # XXX Same here, cron tasks don't start with github + task.get('extra', {}).get('tasks_for', '').startswith('github-'), + is_github_url(task.get('metadata', {}).get('source', '')), )) diff --git a/scriptworker/test/test_cot_verify.py b/scriptworker/test/test_cot_verify.py index 61500d01..cdabb2a8 100644 --- a/scriptworker/test/test_cot_verify.py +++ b/scriptworker/test/test_cot_verify.py @@ -17,8 +17,9 @@ import scriptworker.cot.verify as cotverify from scriptworker.artifacts import get_single_upstream_artifact_full_path from scriptworker.exceptions import CoTError, ScriptWorkerGPGException, DownloadError +from scriptworker.test import create_async, create_finished_future from scriptworker.utils import makedirs, load_json_or_yaml -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from . import noop_async, noop_sync, rw_context, mobile_rw_context, tmpdir, touch @@ -426,13 +427,14 @@ async def test_get_all_links_in_chain(chain, decision_link, build_link): # is_try {{{1 +@pytest.mark.asyncio @pytest.mark.parametrize("bools,expected", (([False, False], False), ([False, True], True))) -def test_chain_is_try_or_pull_request(chain, bools, expected): +async def test_chain_is_try_or_pull_request(chain, bools, expected): for b in bools: m = mock.MagicMock() - m.is_try_or_pull_request = b + m.is_try_or_pull_request = create_async(b) chain.links.append(m) - assert chain.is_try_or_pull_request() == expected + assert await chain.is_try_or_pull_request() == expected # get_link {{{1 @@ -455,10 +457,11 @@ def test_get_link(chain, ids, req, raises): # link.task {{{1 -def test_link_task(chain): +@pytest.mark.asyncio +async def test_link_task(chain): link = cotverify.LinkOfTrust(chain.context, 'build', "one") link.task = chain.task - assert not link.is_try_or_pull_request + assert not await link.is_try_or_pull_request() assert link.worker_impl == 'scriptworker' with pytest.raises(CoTError): link.task = {} @@ -2258,13 +2261,15 @@ async def test_trace_back_to_tree_diff_repo(chain, decision_link, async def test_trace_back_to_tree_mobile_staging_repos_dont_access_restricted_scopes( mobile_chain, mobile_github_release_link, mobile_build_link, source_url, raises ): + (source_url, raises) = ('https://github.com/mozilla-mobile/focus-android', False) mobile_github_release_link.task['metadata']['source'] = source_url mobile_chain.links = [mobile_github_release_link, mobile_build_link] - if raises: - with pytest.raises(CoTError): + with patch.object(mobile_chain, 'is_try_or_pull_request', return_value=create_finished_future(False)): + if raises: + with pytest.raises(CoTError): + await cotverify.trace_back_to_tree(mobile_chain) + else: await cotverify.trace_back_to_tree(mobile_chain) - else: - await cotverify.trace_back_to_tree(mobile_chain) diff --git a/scriptworker/test/test_github.py b/scriptworker/test/test_github.py index c611311d..1ab340c6 100644 --- a/scriptworker/test/test_github.py +++ b/scriptworker/test/test_github.py @@ -1,16 +1,28 @@ +from types import SimpleNamespace + import pytest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from scriptworker import github from scriptworker.context import Context +from scriptworker.exceptions import ConfigError + + +@pytest.yield_fixture(scope='function') +def context(): + yield Context() @pytest.yield_fixture(scope='function') def github_repository(mocker): github_repository_mock = MagicMock() + github_repository_mock.html_url = 'https://github.com/some-user/some-repo/' + github_repository_mock.tags.return_value = [ + SimpleNamespace(name='v1.0.0', commit=SimpleNamespace(sha='hashforv100')), + ] github_instance_mock = MagicMock() - github_instance_mock.repository = github_repository_mock + github_instance_mock.repository.return_value = github_repository_mock mocker.patch.object(github, 'GitHub', return_value=github_instance_mock) yield github.GitHubRepository('some-user', 'some-repo') @@ -31,7 +43,8 @@ def test_constructor(mocker, args, expected_class_kwargs): def test_get_definition(github_repository): - github_repository.definition + github_repository._github_repository.as_dict.return_value = {'foo': 'bar'} + assert github_repository.definition == {'foo': 'bar'} github_repository._github_repository.as_dict.assert_called_once_with() @@ -48,3 +61,195 @@ def test_get_pull_request(github_repository): def test_get_release(github_repository): github_repository.get_release('some-tag') github_repository._github_repository.release_from_tag.assert_called_once_with('some-tag') + + +@pytest.mark.parametrize('tags, raises, expected', (( + [SimpleNamespace( + name='some-tag', + commit=SimpleNamespace(sha='somecommit'), + )], + False, + 'somecommit', +), ( + [SimpleNamespace( + name='another-tag', + commit=SimpleNamespace(sha='anothercommit'), + ), SimpleNamespace( + name='some-tag', + commit=SimpleNamespace(sha='somecommit'), + )], + False, + 'somecommit', +), ( + [SimpleNamespace( + name='another-tag', + commit=SimpleNamespace(sha='anothercommit'), + )], + True, + None +), ( + [], + True, + None, +))) +def test_get_tag_hash(github_repository, tags, raises, expected): + github_repository._github_repository.tags.return_value = tags + + if raises: + with pytest.raises(ValueError): + github_repository.get_tag_hash('some-tag') + else: + assert github_repository.get_tag_hash('some-tag') == expected + + + +@pytest.mark.parametrize('commitish, expected_url, html_text, raises, expected', (( + '0123456789abcdef0123456789abcdef01234567', + 'https://github.com/some-user/some-repo/branch_commits/0123456789abcdef0123456789abcdef01234567', + '\r\n\r\n', + False, + False, +), ( + '0123456789abcdef0123456789abcdef01234567', + 'https://github.com/some-user/some-repo/branch_commits/0123456789abcdef0123456789abcdef01234567', + '\n', + False, + False, +), ( + '0123456789abcdef0123456789abcdef01234567', + 'https://github.com/some-user/some-repo/branch_commits/0123456789abcdef0123456789abcdef01234567', + '', + False, + False, +), ( + '0123456789abcdef0123456789abcdef01234567', + 'https://github.com/some-user/some-repo/branch_commits/0123456789abcdef0123456789abcdef01234567', + ''' + + + + + ''', + False, + True, +), ( + 'v1.0.0', + 'https://github.com/some-user/some-repo/branch_commits/hashforv100', + ''' + + + + + ''', + False, + True, +), ( + 'non-existing-tag', + None, + '', + True, + None, +))) +@pytest.mark.asyncio +async def test_has_commit_landed_on_repository(context, github_repository, commitish, expected_url, html_text, raises, expected): + async def retry_request(_, url): + assert url == expected_url + return html_text + + with patch('scriptworker.github.retry_request', retry_request): + if raises: + with pytest.raises(ValueError): + await github_repository.has_commit_landed_on_repository(context, commitish) + else: + assert await github_repository.has_commit_landed_on_repository(context, commitish) == expected + + +@pytest.mark.parametrize('url, expected', (( + 'https://github.com/', True +), ( + 'https://github.com/some-user', True +), ( + 'https://github.com/some-user/some-repo', True +), ( + 'https://github.com/some-user/some-repo/raw/somerevision/.taskcluster.yml', True +), ( + 'https://hg.mozilla.org', False +), ( + None, False +))) +def test_is_github_url(url, expected): + assert github.is_github_url(url) == expected + + +@pytest.mark.parametrize('repo_url, expected_user, expected_repo_name, raises', (( + 'https://github.com/mozilla-mobile/android-components', + 'mozilla-mobile', 'android-components', False +), ( + 'https://github.com/mozilla-mobile/android-components.git', + 'mozilla-mobile', 'android-components', False +), ( + 'https://github.com/JohanLorenzo/android-components', + 'JohanLorenzo', 'android-components', False +), ( + 'https://github.com/JohanLorenzo/android-components/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml', + 'JohanLorenzo', 'android-components', False +), ( + 'https://hg.mozilla.org/mozilla-central', + None, None, True +))) +def test_extract_github_repo_owner_and_name(repo_url, expected_user, expected_repo_name, raises): + if raises: + with pytest.raises(ValueError): + github.extract_github_repo_owner_and_name(repo_url) + else: + assert github.extract_github_repo_owner_and_name(repo_url) == (expected_user, expected_repo_name) + + +@pytest.mark.parametrize('repo_url, expected_user, expected_repo_name, raises', (( + 'https://github.com/JohanLorenzo/android-components/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml', + 'https://github.com/JohanLorenzo/android-components', '0123456789abcdef0123456789abcdef01234567', False, +), ( + 'https://github.com/JohanLorenzo/android-components.git/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml', + 'https://github.com/JohanLorenzo/android-components', '0123456789abcdef0123456789abcdef01234567', False, +), ( + 'https://github.com/mozilla-mobile/android-components', + None, None, True, +), ( + 'https://github.com/mozilla-mobile/android-components.git', + None, None, True, +), ( + 'https://github.com/JohanLorenzo/android-components', + None, None, True, +), ( + 'https://hg.mozilla.org/mozilla-central', + None, None, True, +))) +def test_extract_github_repo_and_revision_from_source_url(repo_url, expected_user, expected_repo_name, raises): + if raises: + with pytest.raises(ValueError): + github.extract_github_repo_and_revision_from_source_url(repo_url) + else: + assert github.extract_github_repo_and_revision_from_source_url(repo_url) == (expected_user, expected_repo_name) + + +@pytest.mark.parametrize('config_repo_owner, repo_owner, raises, expected', ( + ('mozilla-mobile', 'mozilla-mobile', False, True,), + ('mozilla-mobile', 'JohanLorenzo', False, False,), + ('mozilla-mobile', '', False, False,), + ('', '', True, None,), + ('', 'mozilla-mobile', True, None,), +)) +def test_is_github_repo_owner_the_official_one(context, config_repo_owner, repo_owner, raises, expected): + context.config = {'official_github_repos_owner': config_repo_owner} + + if raises: + with pytest.raises(ConfigError): + github.is_github_repo_owner_the_official_one(context, repo_owner) + else: + assert github.is_github_repo_owner_the_official_one(context, repo_owner) == expected diff --git a/scriptworker/test/test_task.py b/scriptworker/test/test_task.py index 39ef1e2d..a7a516dc 100644 --- a/scriptworker/test/test_task.py +++ b/scriptworker/test/test_task.py @@ -17,6 +17,7 @@ import sys import taskcluster.exceptions import time +from unittest.mock import MagicMock from scriptworker.task_process import TaskProcess from . import fake_session, fake_session_500, noop_async, rw_context, mobile_rw_context, \ @@ -185,30 +186,6 @@ def test_get_push_date_time(push_date_time): assert swtask.get_push_date_time(task, 'MOBILE') == push_date_time -@pytest.mark.parametrize('repo_url, expected_user, expected_repo_name, raises', (( - 'https://github.com/mozilla-mobile/android-components', - 'mozilla-mobile', 'android-components', False -), ( - 'https://github.com/mozilla-mobile/android-components.git', - 'mozilla-mobile', 'android-components', False -), ( - 'https://github.com/JohanLorenzo/android-components', - 'JohanLorenzo', 'android-components', False -), ( - 'https://github.com/JohanLorenzo/android-components/raw/0e8222165d3ec11e932d5f900e8851dc98f62e98/.taskcluster.yml', - 'JohanLorenzo', 'android-components', False -), ( - 'https://hg.mozilla.org/mozilla-central', - None, None, True -))) -def test_extract_github_repo_owner_and_name(repo_url, expected_user, expected_repo_name, raises): - if raises: - with pytest.raises(ValueError): - swtask.extract_github_repo_owner_and_name(repo_url) - else: - assert swtask.extract_github_repo_owner_and_name(repo_url) == (expected_user, expected_repo_name) - - # get_worker_type {{{1 @pytest.mark.parametrize("task,result", (({"workerType": "one"}, "one"), ({"workerType": "two"}, "two"))) def test_get_worker_type(task, result): @@ -340,60 +317,36 @@ def test_is_try(task,source_env_prefix): assert swtask.is_try(task, source_env_prefix=source_env_prefix) -@pytest.mark.parametrize('task, expected', (( - {'payload': {}, 'extra': {'env': {'tasks_for': 'github-pull-request'}}, 'metadata': {}, }, - True, -), ( - {'payload': {'env': {'MOBILE_HEAD_REPOSITORY': 'https://github.com/some-user/some-repo'}}, 'extra': {'env': {'tasks_for': 'github-push'}}, 'metadata': {}}, - True, -), ( - {'payload': {'env': {'MOBILE_HEAD_REPOSITORY': 'https://github.com/some-user/some-repo.git'}}, 'extra': {'env': {'tasks_for': 'github-release'}}, 'metadata': {}}, - True, -), ( - {'payload': {'env': {'MOBILE_HEAD_REPOSITORY': 'https://github.com/some-user/some-repo.git'}}, 'metadata': {}}, - True, -), ( - {'payload': {}, 'metadata': {'source': 'https://github.com/some-user/some-repo'}}, - True, -), ( - {'payload': {}, 'metadata': {'source': 'https://github.com/some-user/some-repo/raw/somerevision/.taskcluster.yml'}}, - True, +@pytest.mark.parametrize('task, has_commit_landed, raises, expected', (( + { + 'payload': {}, + 'extra': {'env': {'tasks_for': 'github-pull-request'}}, + 'metadata': {'source': 'https://github.com/some-user/some-repo/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml'}, + }, + True, False, True, ), ( { - 'extra': { - 'env': { - 'tasks_for': 'cron', - }, - }, - 'metadata': { - 'source': 'https://github.com/mozilla-mobile/some-repo/raw/some-revision/.taskcluster.yml', - }, 'payload': { - 'env': { - 'MOBILE_HEAD_REPOSITORY': 'https://github.com/mozilla-mobile/some-repo', - }, + 'env': {'MOBILE_HEAD_REPOSITORY': 'https://github.com/some-user/some-repo'} }, + 'extra': {'env': {'tasks_for': 'github-push'}}, + 'metadata': {'source': 'https://github.com/some-user/some-repo/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml'}, }, - False -))) -def test_is_pull_request(mobile_context, task, expected): - assert swtask.is_pull_request(mobile_context, task) == expected - - -@pytest.mark.parametrize('context_type, task, expected', (( -# 'firefox', -# {'payload': {'env': {'GECKO_HEAD_REPOSITORY': 'https://hg.mozilla.org/try/blahblah'}}, 'metadata': {}, 'schedulerId': 'x'}, -# True, -# ), ( -# 'mobile', -# {'payload': {}, 'extra': {'env': {'tasks_for': 'github-pull-request'}}, 'metadata': {}, 'schedulerId': 'taskcluster-github'}, -# True, -# ), ( - 'firefox', - {'payload': {'env': {'GECKO_HEAD_REPOSITORY': 'https://hg.mozilla.org/mozilla-central'}}, 'metadata': {}, 'schedulerId': 'x'}, - False, -), ( - 'mobile', + True, False, True, +), ( + { + 'payload': {'env': {'MOBILE_HEAD_REPOSITORY': 'https://github.com/some-user/some-repo.git'}}, + 'extra': {'env': {'tasks_for': 'github-release'}}, + 'metadata': {'source': 'https://github.com/some-user/some-repo/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml'}, + }, + True, False, True, +), ( + { + 'payload': {}, + 'metadata': {'source': 'https://github.com/some-user/some-repo/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml'}, + }, + True, False, True, +), ( { 'extra': { 'env': { @@ -401,20 +354,87 @@ def test_is_pull_request(mobile_context, task, expected): }, }, 'metadata': { - 'source': 'https://github.com/mozilla-mobile/some-repo/raw/some-revision/.taskcluster.yml', + 'source': 'https://github.com/mozilla-mobile/some-repo/raw/0123456789abcdef0123456789abcdef01234567/.taskcluster.yml', }, 'payload': { 'env': { 'MOBILE_HEAD_REPOSITORY': 'https://github.com/mozilla-mobile/some-repo', }, }, - 'schedulerId': 'taskcluster-github', }, - False + True, False, False, +), ( + {'payload': {'env': {'MOBILE_HEAD_REPOSITORY': 'https://github.com/some-user/some-repo.git'}}, 'metadata': {}}, + True, True, None, +), ( + {'extra': {}, 'metadata': {'source': 'https://some-non-github-url.tld',}, 'payload': {},}, + True, True, None, +), ( + {'payload': {}, 'metadata': {'source': 'https://github.com/some-user/some-repo'}}, + True, True, None, ))) -def test_is_try_or_pull_request(context, mobile_context, context_type, task, expected): +@pytest.mark.asyncio +async def test_is_pull_request(mocker, mobile_context, task, has_commit_landed, raises, expected): + async def has_commit_landed_on_repository(*args): + return has_commit_landed + + github_repository_instance_mock = MagicMock() + github_repository_instance_mock.has_commit_landed_on_repository = has_commit_landed_on_repository + GitHubRepositoryClassMock = MagicMock() + GitHubRepositoryClassMock.return_value = github_repository_instance_mock + mocker.patch.object(swtask, 'GitHubRepository', GitHubRepositoryClassMock) + + if raises: + with pytest.raises(ValueError): + await swtask.is_pull_request(mobile_context, task) + else: + assert await swtask.is_pull_request(mobile_context, task) == expected + + +@pytest.mark.parametrize('context_type, is_github_task, is_try, is_pr, expected', ( + ('firefox', True, True, False, False,), + ('firefox', True, False, False, False,), + ('firefox', False, False, False, False,), + ('firefox', False, True, False, True,), + + ('mobile', True, False, True, True,), + ('mobile', True, False, False, False,), + ('mobile', False, False, False, False,), + ('mobile', False, False, True, False,), +)) +@pytest.mark.asyncio +async def test_is_try_or_pull_request(mocker, context, mobile_context, context_type, is_github_task, is_try, is_pr, expected): context_ = mobile_context if context_type == 'mobile' else context - assert swtask.is_try_or_pull_request(context_, task) == expected + + async def is_pull_request(*args): + return is_pr + + mocker.patch.object(swtask, 'is_github_task', lambda *args: is_github_task) + mocker.patch.object(swtask, 'is_pull_request', is_pull_request) + mocker.patch.object(swtask, 'is_try', lambda *args: is_try) + + assert await swtask.is_try_or_pull_request(context_, {}) == expected + + +@pytest.mark.parametrize('task, expected', (( + {'schedulerId': 'taskcluster-github'}, True, +), ( + {'extra': {'tasks_for': 'github-pull-request'}}, True, +), ( + {'extra': {'tasks_for': 'github-push'}}, True, +), ( + {'extra': {'tasks_for': 'github-release'}}, True, +), ( + {'metadata': {'source': 'https://github.com/some-owner/some-repo'}}, True, +), ( + {'extra': {'tasks_for': 'cron'}, 'metadata': {'source': 'https://github.com/some-owner/some-repo'}}, True, +), ( + {'schedulerId': 'gecko-level-1', 'extra': {'tasks_for': 'hg-push'}, 'metadata': {'source': 'https://hg.mozilla.org/try'}}, False, +), ( + {'schedulerId': 'gecko-level-3', 'extra': {'tasks_for': 'action'}, 'metadata': {'source': 'https://hg.mozilla.org/mozilla-central'}}, False, +))) +def test_is_github_task(task, expected): + assert swtask.is_github_task(task) == expected # is_action {{{1 diff --git a/scriptworker/test/test_utils.py b/scriptworker/test/test_utils.py index 3e428a16..13b96754 100644 --- a/scriptworker/test/test_utils.py +++ b/scriptworker/test/test_utils.py @@ -413,6 +413,19 @@ def test_get_loggable_url(url, expected): assert utils.get_loggable_url(url) == expected +@pytest.mark.parametrize('url, expected', (( + 'https://foo/bar', ['bar'] +), ( + 'https://foo/bar/baz', ['bar', 'baz'] +), ( + 'https://foo/bar/baz?param1=value', ['bar', 'baz'] +), ( + 'https://foo/bar/baz?param1=value1¶m2=value2', ['bar', 'baz'] +))) +def test_get_parts_of_url_path(url, expected): + assert utils.get_parts_of_url_path(url) == expected + + # match_url_path_callback {{{1 @pytest.mark.parametrize("path", ( "/mozilla-central", "/mozilla-central/foo/bar", "/mozilla-central/" diff --git a/scriptworker/utils.py b/scriptworker/utils.py index 7d1c0e0c..37cf032a 100644 --- a/scriptworker/utils.py +++ b/scriptworker/utils.py @@ -527,6 +527,22 @@ def get_loggable_url(url): return loggable_url +def get_parts_of_url_path(url): + """Given a url, take out the path part and split it by '/'. + + Args: + url (str): the url slice + + returns + list: parts after the domain name of the URL + + """ + parsed = urlparse(url) + path = unquote(parsed.path).lstrip('/') + parts = path.split('/') + return parts + + # load_json_or_yaml_from_url {{{1 async def load_json_or_yaml_from_url(context, url, path, overwrite=True): """Retry a json/yaml file download, load it, then return its data.