From ead2cb6a59844d8cd779fb014c0e16178ae15964 Mon Sep 17 00:00:00 2001 From: Johan Lorenzo Date: Fri, 17 Apr 2026 18:14:13 +0200 Subject: [PATCH 1/2] landoscript: extend version_bump to handle JSON manifests (bug 2025695) --- .../src/landoscript/actions/version_bump.py | 15 ++- landoscript/tests/test_version_bump.py | 94 +++++++++++++++++++ .../tests/test_version_bump_json_manifest.py | 33 +++++++ 3 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 landoscript/tests/test_version_bump_json_manifest.py diff --git a/landoscript/src/landoscript/actions/version_bump.py b/landoscript/src/landoscript/actions/version_bump.py index d8f60641a..bb70f9c52 100644 --- a/landoscript/src/landoscript/actions/version_bump.py +++ b/landoscript/src/landoscript/actions/version_bump.py @@ -1,3 +1,4 @@ +import json import logging import os.path import typing @@ -17,10 +18,11 @@ log = logging.getLogger(__name__) -# A list of files that this action is allowed to operate on. ALLOWED_BUMP_FILES = ( "browser/config/version.txt", "browser/config/version_display.txt", + "browser/extensions/newtab/manifest.json", + "browser/extensions/webcompat/manifest.json", "config/milestone.txt", "mobile/android/version.txt", "mail/config/version.txt", @@ -34,6 +36,12 @@ class VersionBumpInfo: files: list[str] +def parse_manifest_version(orig_contents: str) -> BaseVersion: + """Extract and parse the version field from a JSON manifest.""" + manifest = json.loads(orig_contents) + return BaseVersion.parse(manifest["version"]) + + async def run( github_client: GithubClient, public_artifact_dir: str, @@ -119,6 +127,11 @@ def extract_path(diff_text): def get_cur_and_next_version(filename, orig_contents, next_version, munge_next_version): + if filename.endswith(".json"): + cur = parse_manifest_version(orig_contents) + next_ = BaseVersion.parse(next_version) + return cur, next_ + VersionClass: BaseVersion = find_what_version_parser_to_use(filename) lines = [line for line in orig_contents.splitlines() if line and not line.startswith("#")] cur = VersionClass.parse(lines[-1]) diff --git a/landoscript/tests/test_version_bump.py b/landoscript/tests/test_version_bump.py index 8ec84f8dd..7cb1e972b 100644 --- a/landoscript/tests/test_version_bump.py +++ b/landoscript/tests/test_version_bump.py @@ -6,6 +6,7 @@ from landoscript.errors import LandoscriptError from landoscript.script import async_main from landoscript.actions.version_bump import ALLOWED_BUMP_FILES +import json as _json from landoscript.util.version import _VERSION_CLASS_PER_BEGINNING_OF_PATH from .conftest import ( @@ -394,4 +395,97 @@ def test_no_overlaps_in_version_classes(): def test_all_bump_files_have_version_class(): for bump_file in ALLOWED_BUMP_FILES: + # JSON manifests use BaseVersion directly and bypass the version class lookup + if bump_file.endswith(".json"): + continue assert any([bump_file.startswith(path) for path in _VERSION_CLASS_PER_BEGINNING_OF_PATH]) + + +NEWTAB_MANIFEST = "browser/extensions/newtab/manifest.json" +WEBCOMPAT_MANIFEST = "browser/extensions/webcompat/manifest.json" +MANIFEST_FILE = NEWTAB_MANIFEST + + +def _manifest(version): + return _json.dumps({"manifest_version": 2, "name": "New Tab", "version": version}, indent=2) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "initial_version,expected_version", + [ + pytest.param("151.0.0", "151.1.0", id="initial_after_merge_day"), + pytest.param("151.1.0", "151.2.0", id="normal_minor_bump"), + ], +) +async def test_json_manifest_bump(aioresponses, github_installation_responses, context, initial_version, expected_version): + payload = { + "actions": ["version_bump"], + "lando_repo": "repo_name", + "version_bump_info": { + "files": [MANIFEST_FILE], + "next_version": expected_version, + }, + } + setup_github_graphql_responses(aioresponses, get_files_payload({MANIFEST_FILE: _manifest(initial_version)})) + await run_test( + aioresponses, + github_installation_responses, + context, + payload, + ["version_bump"], + assert_func=lambda req: assert_success( + req, + ["Automatic version bump", "NO BUG", "a=release", "CLOSED TREE"], + {MANIFEST_FILE: f' "version": "{initial_version}"'}, + {MANIFEST_FILE: f' "version": "{expected_version}"'}, + ), + ) + + +@pytest.mark.asyncio +async def test_json_manifest_file_not_found(aioresponses, github_installation_responses, context): + payload = { + "actions": ["version_bump"], + "lando_repo": "repo_name", + "version_bump_info": { + "files": [MANIFEST_FILE], + "next_version": "151.1.0", + }, + } + setup_github_graphql_responses(aioresponses, get_files_payload({MANIFEST_FILE: None})) + await run_test( + aioresponses, + github_installation_responses, + context, + payload, + ["version_bump"], + err=LandoscriptError, + errmsg="does not exist", + ) + + +@pytest.mark.asyncio +async def test_webcompat_manifest_bump(aioresponses, github_installation_responses, context): + payload = { + "actions": ["version_bump"], + "lando_repo": "repo_name", + "version_bump_info": { + "files": [WEBCOMPAT_MANIFEST], + "next_version": "151.1.0", + }, + } + setup_github_graphql_responses(aioresponses, get_files_payload({WEBCOMPAT_MANIFEST: _manifest("151.0.0")})) + await run_test( + aioresponses, + github_installation_responses, + context, + payload, + ["version_bump"], + assert_func=lambda req: assert_success( + req, + ["Automatic version bump", "NO BUG", "a=release", "CLOSED TREE"], + {WEBCOMPAT_MANIFEST: ' "version": "151.0.0"'}, + {WEBCOMPAT_MANIFEST: ' "version": "151.1.0"'}, + ), + ) diff --git a/landoscript/tests/test_version_bump_json_manifest.py b/landoscript/tests/test_version_bump_json_manifest.py new file mode 100644 index 000000000..c243e04be --- /dev/null +++ b/landoscript/tests/test_version_bump_json_manifest.py @@ -0,0 +1,33 @@ +import json + +import pytest + +from landoscript.actions.version_bump import parse_manifest_version + + +def _manifest(version, name="New Tab"): + return json.dumps({"manifest_version": 2, "name": name, "version": version}, indent=2) + + +@pytest.mark.parametrize( + "version", + ["151.0.0", "151.1.0", "151.2.0"], +) +def test_parse_manifest_version(version): + result = parse_manifest_version(_manifest(version)) + assert str(result) == version + + +def test_parse_manifest_version_ignores_other_fields(): + contents = '{"manifest_version": 2, "version": "151.0.0", "strict_min_version": "140.0"}' + result = parse_manifest_version(contents) + assert str(result) == "151.0.0" + + +@pytest.mark.parametrize( + "manifest_name", + ["New Tab", "Web Compat"], +) +def test_parse_manifest_version_works_for_newtab_and_webcompat(manifest_name): + result = parse_manifest_version(_manifest("151.0.0", name=manifest_name)) + assert str(result) == "151.0.0" From a6a7bf1d1a76b9fb2e446cab766a5b1b111a8baa Mon Sep 17 00:00:00 2001 From: Johan Lorenzo Date: Fri, 17 Apr 2026 18:14:49 +0200 Subject: [PATCH 2/2] landoscript: make version_bump idempotent when file is unchanged (bug 2025695) Skip silently rather than raising if str.replace produces no change, so retried tasks don't fail spuriously if the bump already landed. --- landoscript/src/landoscript/actions/version_bump.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/landoscript/src/landoscript/actions/version_bump.py b/landoscript/src/landoscript/actions/version_bump.py index bb70f9c52..7c8ccdbe5 100644 --- a/landoscript/src/landoscript/actions/version_bump.py +++ b/landoscript/src/landoscript/actions/version_bump.py @@ -95,7 +95,8 @@ async def run( modified = orig.replace(str(cur), str(next_)) if orig == modified: - raise LandoscriptError("file not modified, this should be impossible") + log.warning(f"{file}: version replacement produced no change, skipping") + continue log.info(f"{file}: successfully bumped! new contents are:") log_file_contents(modified)