From a5877b180ae9b9c00bb22a5a3585e2711c3e5832 Mon Sep 17 00:00:00 2001 From: Ben Hearsum Date: Thu, 17 Apr 2025 13:15:00 -0400 Subject: [PATCH] fix(landoscript): check all android toml files at one time, before processing any of them This is a follow to address something [@ahal called out in the initial review of landoscript](https://github.com/mozilla-releng/scriptworker-scripts/pull/1135#discussion_r2047655019). --- .../actions/android_l10n_import.py | 7 +- .../landoscript/actions/android_l10n_sync.py | 7 +- landoscript/tests/test_android_l10n_import.py | 66 ++++++++++++++++++- landoscript/tests/test_android_l10n_sync.py | 45 +++++++++++++ 4 files changed, 118 insertions(+), 7 deletions(-) diff --git a/landoscript/src/landoscript/actions/android_l10n_import.py b/landoscript/src/landoscript/actions/android_l10n_import.py index b71cf83a4..81260f325 100644 --- a/landoscript/src/landoscript/actions/android_l10n_import.py +++ b/landoscript/src/landoscript/actions/android_l10n_import.py @@ -51,13 +51,14 @@ async def run( toml_contents = await l10n_github_client.get_files(toml_files) l10n_files: list[L10nFile] = [] + missing = [fn for fn, contents in toml_contents.items() if contents is None] + if missing: + raise LandoscriptError(f"toml_file(s) {' '.join(missing)} are not present in repository") + for info in android_l10n_import_info.toml_info: toml_file = info.toml_path log.info(f"processing toml file: {toml_file}") - if toml_contents[toml_file] is None: - raise LandoscriptError(f"toml_file '{toml_file}' is not present in repository") - contents = tomllib.loads(str(toml_contents[toml_file])) src_file_prefix = Path(toml_file).parent dst_file_prefix = Path(info.dest_path) diff --git a/landoscript/src/landoscript/actions/android_l10n_sync.py b/landoscript/src/landoscript/actions/android_l10n_sync.py index 8cb1a0473..fe81e95ea 100644 --- a/landoscript/src/landoscript/actions/android_l10n_sync.py +++ b/landoscript/src/landoscript/actions/android_l10n_sync.py @@ -43,13 +43,14 @@ async def run(github_client: GithubClient, public_artifact_dir: str, android_l10 toml_contents = await github_client.get_files(toml_files, branch=from_branch) l10n_files: list[L10nFile] = [] + missing = [fn for fn, contents in toml_contents.items() if contents is None] + if missing: + raise LandoscriptError(f"toml_file(s) {' '.join(missing)} are not present in repository") + for info in android_l10n_sync_info.toml_info: toml_file = info.toml_path log.info(f"processing toml file: {toml_file}") - if toml_contents[toml_file] is None: - raise LandoscriptError(f"toml_file '{toml_file}' is not present in repository") - contents = tomllib.loads(str(toml_contents[toml_file])) src_file_prefix = Path(toml_file).parent dst_file_prefix = src_file_prefix diff --git a/landoscript/tests/test_android_l10n_import.py b/landoscript/tests/test_android_l10n_import.py index 2bba164bf..0be1881d9 100644 --- a/landoscript/tests/test_android_l10n_import.py +++ b/landoscript/tests/test_android_l10n_import.py @@ -1,5 +1,7 @@ import pytest +from yarl import URL +from landoscript.errors import LandoscriptError from landoscript.script import async_main from tests.conftest import ( assert_add_commit_response, @@ -241,7 +243,9 @@ async def test_success( "lando_repo": "repo_name", "android_l10n_import_info": android_l10n_import_info, } - # done here because setup_test sets up github_installation_response too soon...argh + # this is the same setup that's done in `setup_test`, but in a slightly different + # order to accommodate the fact that we query the `mozilla-l10n` repository before + # the `lando_repo` repository. from yarl import URL lando_repo = payload["lando_repo"] @@ -309,3 +313,63 @@ async def test_success( else: assert ("POST", submit_uri) not in aioresponses.requests assert ("GET", status_uri) not in aioresponses.requests + + +@pytest.mark.asyncio +async def test_missing_toml_file(aioresponses, github_installation_responses, context): + payload = { + "actions": ["android_l10n_import"], + "lando_repo": "repo_name", + "android_l10n_import_info": { + "from_repo_url": "https://github.com/mozilla-l10n/android-l10n", + "toml_info": [ + { + "dest_path": "mobile/android/fenix", + "toml_path": "mozilla-mobile/fenix/l10n.toml", + }, + { + "dest_path": "mobile/android/focus-android", + "toml_path": "mozilla-mobile/focus-android/l10n.toml", + }, + { + "dest_path": "mobile/android/android-components", + "toml_path": "mozilla-mobile/android-components/l10n.toml", + }, + ], + }, + } + + scopes = [f"project:releng:lando:repo:repo_name"] + scopes.append(f"project:releng:lando:action:android_l10n_import") + + lando_api = context.config["lando_api"] + repo_info_uri = URL(f"{lando_api}/api/repoinfo/repo_name") + aioresponses.get( + repo_info_uri, + status=200, + payload={ + "repo_url": f"https://github.com/faker/repo_name", + "branch_name": "fake_branch", + "scm_level": "whatever", + }, + ) + + github_installation_responses("mozilla-l10n") + setup_github_graphql_responses( + aioresponses, + # toml files needed before fetching anything else + fetch_files_payload( + { + "mozilla-mobile/fenix/l10n.toml": None, + "mozilla-mobile/focus-android/l10n.toml": focus_l10n_toml, + "mozilla-mobile/android-components/l10n.toml": ac_l10n_toml, + } + ), + ) + + context.task = {"payload": payload, "scopes": scopes} + try: + await async_main(context) + assert False, "should've raised LandoscriptError" + except LandoscriptError as e: + assert "toml_file(s) mozilla-mobile/fenix/l10n.toml are not present" in e.args[0] diff --git a/landoscript/tests/test_android_l10n_sync.py b/landoscript/tests/test_android_l10n_sync.py index 83481e6ff..597439807 100644 --- a/landoscript/tests/test_android_l10n_sync.py +++ b/landoscript/tests/test_android_l10n_sync.py @@ -1,6 +1,7 @@ import pytest from scriptworker_client.github_client import TransportQueryError +from landoscript.errors import LandoscriptError from tests.conftest import ( assert_add_commit_response, get_file_listing_payload, @@ -246,3 +247,47 @@ def assert_func(req): should_submit = True await run_test(aioresponses, github_installation_responses, context, payload, ["android_l10n_sync"], should_submit, assert_func) + + +@pytest.mark.asyncio +async def test_missing_toml_file(aioresponses, github_installation_responses, context): + payload = { + "actions": ["android_l10n_sync"], + "lando_repo": "repo_name", + "android_l10n_sync_info": { + "from_branch": "main", + "toml_info": [ + { + "toml_path": "mobile/android/fenix/l10n.toml", + }, + { + "toml_path": "mobile/android/focus-android/l10n.toml", + }, + { + "toml_path": "mobile/android/android-components/l10n.toml", + }, + ], + }, + } + + setup_github_graphql_responses( + aioresponses, + # toml files needed before fetching anything else + fetch_files_payload( + { + "mobile/android/fenix/l10n.toml": None, + "mobile/android/focus-android/l10n.toml": focus_l10n_toml, + "mobile/android/android-components/l10n.toml": ac_l10n_toml, + } + ), + ) + + await run_test( + aioresponses, + github_installation_responses, + context, + payload, + ["android_l10n_sync"], + err=LandoscriptError, + errmsg="toml_file(s) mobile/android/fenix/l10n.toml are not present", + )