Skip to content

Commit

Permalink
fix(cot): only use parent repo for pull request data if base repo dif…
Browse files Browse the repository at this point in the history
…fers from head repo
  • Loading branch information
ahal committed May 21, 2024
1 parent f90867d commit adbb1b1
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 15 deletions.
8 changes: 7 additions & 1 deletion src/scriptworker/cot/verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,8 @@ async def _get_additional_github_pull_request_jsone_context(decision_link):
context = decision_link.context
source_env_prefix = context.config["source_env_prefix"]
task = decision_link.task
base_repo_url = get_repo(task, source_env_prefix, repo_type="base")
base_repo_url = base_repo_url.replace("[email protected]:", "ssh://github.com/", 1)
repo_url = get_repo(task, source_env_prefix)
repo_url = repo_url.replace("[email protected]:", "ssh://github.com/", 1)
repo_owner, repo_name = extract_github_repo_owner_and_name(repo_url)
Expand All @@ -1176,7 +1178,11 @@ async def _get_additional_github_pull_request_jsone_context(decision_link):
github_repo = GitHubRepository(repo_owner, repo_name, token)
repo_definition = github_repo.definition

if repo_definition["fork"]:
# We need to query the repository where the pull request was made to extract
# pull request data. The pull request could be created on the same repo as
# the commit, or an upstream repo. We can compare the base and head repo URLs
# to infer where the pull request lives.
if repo_definition["fork"] and base_repo_url != repo_url:
github_repo = GitHubRepository(owner=repo_definition["parent"]["owner"]["login"], repo_name=repo_definition["parent"]["name"], token=token)

pull_request_data = await github_repo.get_pull_request(pull_request_number)
Expand Down
4 changes: 2 additions & 2 deletions src/scriptworker/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def get_parent_task_id(task):


# get_repo {{{1
def get_repo(task, source_env_prefix):
def get_repo(task, source_env_prefix, repo_type="head"):
"""Get the repo for a task.
Args:
Expand All @@ -191,7 +191,7 @@ def get_repo(task, source_env_prefix):
None: if not defined for this task.
"""
repo = _extract_from_env_in_payload(task, source_env_prefix + "_HEAD_REPOSITORY")
repo = _extract_from_env_in_payload(task, f"{source_env_prefix}_{repo_type.upper()}_REPOSITORY")
if repo is not None:
repo = repo.rstrip("/")
return repo
Expand Down
22 changes: 18 additions & 4 deletions tests/test_cot_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ def mobile_github_pull_request_link(mobile_chain):
)
decision_link.task["payload"]["env"] = {
"MOBILE_BASE_REF": "main",
"MOBILE_BASE_REPOSITORY": "https://github.com/mozilla-mobile/firefox-android",
"MOBILE_BASE_REV": "baserev",
"MOBILE_HEAD_REF": "some-branch",
"MOBILE_HEAD_REPOSITORY": "https://github.com/JohanLorenzo/reference-browser",
Expand Down Expand Up @@ -1205,11 +1206,24 @@ async def test_get_additional_git_action_jsone_context(github_action_link):
}


@pytest.mark.parametrize("is_fork", (True, False))
@pytest.mark.parametrize(
"extra_env,extra_repo_definition,expected_use_parent",
(
pytest.param({}, {"fork": True}, True, id="fork with different base repo"),
pytest.param({}, {"fork": False}, False, id="no fork with different base repo"),
pytest.param({"MOBILE_BASE_REPOSITORY": "https://github.com/JohanLorenzo/reference-browser"}, {"fork": True}, False, id="fork with same base repo"),
),
)
@pytest.mark.asyncio
async def test_populate_jsone_context_github_pull_request(mocker, mobile_chain_pull_request, mobile_github_pull_request_link, is_fork):
async def test_populate_jsone_context_github_pull_request(
mocker, mobile_chain_pull_request, mobile_github_pull_request_link, extra_env, extra_repo_definition, expected_use_parent
):
github_repo_mock = MagicMock()
github_repo_mock.definition = {"fork": True, "parent": {"name": "reference-browser", "owner": {"login": "mozilla-mobile"}}} if is_fork else {"fork": False}
repo_definition = {"fork": True, "parent": {"name": "reference-browser", "owner": {"login": "mozilla-mobile"}}}
repo_definition.update(extra_repo_definition)
github_repo_mock.definition = repo_definition

mobile_github_pull_request_link.task["payload"]["env"].update(extra_env)

async def get_pull_request_mock(pull_request_number, *args, **kwargs):
assert pull_request_number == 1234
Expand Down Expand Up @@ -1241,7 +1255,7 @@ async def get_pull_request_mock(pull_request_number, *args, **kwargs):

github_repo_class_mock.assert_any_call("JohanLorenzo", "reference-browser", "fakegithubtoken")

if is_fork:
if expected_use_parent:
github_repo_class_mock.assert_any_call(owner="mozilla-mobile", repo_name="reference-browser", token="fakegithubtoken")
assert len(github_repo_class_mock.call_args_list) == 2
else:
Expand Down
54 changes: 46 additions & 8 deletions tests/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,52 @@ def test_get_parent_task_id(set_parent):


# get_repo {{{1
@pytest.mark.parametrize("repo", (None, "https://hg.mozilla.org/mozilla-central", "https://hg.mozilla.org/mozilla-central/"))
def test_get_repo(repo):
task = {"payload": {"env": {}}}
if repo:
task["payload"]["env"]["GECKO_HEAD_REPOSITORY"] = repo
assert swtask.get_repo(task, "GECKO") == "https://hg.mozilla.org/mozilla-central"
else:
assert swtask.get_repo(task, "GECKO") is None
@pytest.mark.parametrize(
"repo,repo_type,expected",
(
pytest.param(None, None, None),
pytest.param(
"https://hg.mozilla.org/mozilla-central",
None,
"https://hg.mozilla.org/mozilla-central",
),
pytest.param(
"https://hg.mozilla.org/mozilla-central/",
None,
"https://hg.mozilla.org/mozilla-central",
),
pytest.param(
"https://hg.mozilla.org/mozilla-central",
"head",
"https://hg.mozilla.org/mozilla-central",
),
pytest.param(
"https://hg.mozilla.org/mozilla-central",
"BASE",
"https://hg.mozilla.org/mozilla-central",
),
pytest.param(
"https://hg.mozilla.org/mozilla-central",
"unknown",
None,
),
),
)
def test_get_repo(repo, repo_type, expected):
task = {
"payload": {
"env": {
"GECKO_BASE_REPOSITORY": repo,
"GECKO_HEAD_REPOSITORY": repo,
}
}
}

kwargs = {}
if repo_type:
kwargs["repo_type"] = repo_type

assert swtask.get_repo(task, "GECKO", **kwargs) == expected


@pytest.mark.parametrize("rev", (None, "revision!"))
Expand Down

0 comments on commit adbb1b1

Please sign in to comment.