diff --git a/.github/workflows/play-promote.yml b/.github/workflows/play-promote.yml index d5ad318..9849775 100644 --- a/.github/workflows/play-promote.yml +++ b/.github/workflows/play-promote.yml @@ -15,8 +15,8 @@ on: to_track: description: 'track to promote it to (internal, alpha = closed, beta = open, production)' default: beta - clear_track: - description: 'track to leave with no release, in the same edit (blank = none)' + halt_track: + description: 'track to stop serving, in the same edit (blank = none)' default: '' dry_run: description: 'print the plan and commit nothing' @@ -46,7 +46,7 @@ jobs: VERSION_CODE: ${{ inputs.version_code }} FROM_TRACK: ${{ inputs.from_track }} TO_TRACK: ${{ inputs.to_track }} - CLEAR_TRACK: ${{ inputs.clear_track }} + HALT_TRACK: ${{ inputs.halt_track }} DRY_RUN: ${{ inputs.dry_run }} run: | set -euo pipefail @@ -54,7 +54,7 @@ jobs: trap 'rm -f "$RUNNER_TEMP/play-sa.json"' EXIT printf '%s' "$SA_JSON" > "$RUNNER_TEMP/play-sa.json" args=() - if [ -n "$CLEAR_TRACK" ]; then args+=(--clear-track "$CLEAR_TRACK"); fi + if [ -n "$HALT_TRACK" ]; then args+=(--halt-track "$HALT_TRACK"); fi if [ "$DRY_RUN" = "true" ]; then args+=(--dry-run); fi python3 tools/ci/play_track_admin.py "$RUNNER_TEMP/play-sa.json" promote \ "$VERSION_CODE" "$FROM_TRACK" "$TO_TRACK" \ diff --git a/tools/ci/play_track_admin.py b/tools/ci/play_track_admin.py index 9dae924..ef8d702 100644 --- a/tools/ci/play_track_admin.py +++ b/tools/ci/play_track_admin.py @@ -8,7 +8,7 @@ Usage: play_track_admin.py list play_track_admin.py promote - [--notes-dir DIR] [--clear-track TRACK] [--dry-run] + [--notes-dir DIR] [--halt-track TRACK] [--dry-run] Track names are the API's, not the Console's: alpha is Closed testing, beta is Open testing. @@ -127,6 +127,18 @@ def find_release(track, version_code): return None +def halted(track): + """The track's releases with distribution stopped. + + Sending an empty releases list does NOT clear a track: Play accepts the PUT, commits it, + and keeps serving the old release. Flipping each release to `halted` is what stops it. + """ + live = [r for r in track.get("releases") or [] if r.get("status") != "halted"] + if not live: + sys.exit(f"{track['track']} serves nothing to halt") + return [dict(r, status="halted") for r in live] + + def put_track(session, eid, track, releases): """PUT a track body, dropping a locale Play rejects rather than failing the whole run.""" body = {"track": track, "releases": releases} @@ -169,7 +181,7 @@ def main(): ap.add_argument("from_track", nargs="?") ap.add_argument("to_track", nargs="?") ap.add_argument("--notes-dir", help="directory of .txt release notes") - ap.add_argument("--clear-track", help="track to leave with no release, in the same edit") + ap.add_argument("--halt-track", help="track to stop serving, in the same edit") ap.add_argument("--dry-run", action="store_true", help="print the plan, commit nothing") args = ap.parse_args() @@ -223,15 +235,15 @@ def main(): release["releaseNotes"] = src["releaseNotes"] plan = [(args.to_track, [release])] - if args.clear_track: - if args.clear_track not in by_name: - sys.exit(f"no such track: {args.clear_track}") - # Both PUTs share one edit, so clearing the target would just erase the promote. - if args.clear_track == args.to_track: - sys.exit(f"--clear-track {args.clear_track} is also the promote target") - if args.clear_track == "production": - sys.exit("refusing to unpublish production") - plan.append((args.clear_track, [])) + if args.halt_track: + if args.halt_track not in by_name: + sys.exit(f"no such track: {args.halt_track}") + # Both PUTs share one edit, so halting the target would undo the promote. + if args.halt_track == args.to_track: + sys.exit(f"--halt-track {args.halt_track} is also the promote target") + if args.halt_track == "production": + sys.exit("refusing to halt production") + plan.append((args.halt_track, halted(by_name[args.halt_track]))) if args.dry_run: print("=== plan (dry run, nothing committed) ===") diff --git a/tools/ci/play_track_admin_test.py b/tools/ci/play_track_admin_test.py index e03cfbf..b5a3604 100644 --- a/tools/ci/play_track_admin_test.py +++ b/tools/ci/play_track_admin_test.py @@ -30,6 +30,7 @@ "releases": [{"name": "3.49.14.86", "versionCodes": ["86"], "status": "completed"}], }, {"track": "beta", "releases": []}, + {"track": "empty", "releases": []}, {"track": "production", "releases": []}, ] } @@ -62,7 +63,10 @@ class FakeSession: edit, package or path fails here rather than silently passing. """ - def __init__(self, reject_message=None, reject_times=1, fail_after_commit=False): + def __init__( + self, reject_message=None, reject_times=1, fail_after_commit=False, body_less_put=False + ): + self.body_less_put = body_less_put self.fail_after_commit = fail_after_commit self.headers = {} self.puts = [] @@ -102,7 +106,7 @@ def put(self, url, json=None, **kw): self.reject_times -= 1 return FakeResponse(400, {"error": {"message": self.reject_message}}) self.puts.append((track, copy.deepcopy(json))) - return FakeResponse(200) if not json["releases"] else FakeResponse(200, json) + return FakeResponse(200) if self.body_less_put else FakeResponse(200, json) def delete(self, url, **kw): self.deleted.append(self._edit_id(url)) @@ -140,7 +144,7 @@ def promote(*extra, notes=None): class Test(unittest.TestCase): def test_promote_and_clear_share_one_edit(self): s = FakeSession() - run(promote("--clear-track", "alpha"), s) + run(promote("--halt-track", "alpha"), s) self.assertEqual([t for t, _ in s.puts], ["beta", "alpha"]) beta = dict(s.puts)["beta"]["releases"][0] self.assertEqual(beta["versionCodes"], ["89"]) @@ -148,7 +152,10 @@ def test_promote_and_clear_share_one_edit(self): self.assertNotIn("userFraction", beta) self.assertEqual(beta["name"], "3.50-beta-1") # carried over from the source release self.assertEqual({n["language"] for n in beta["releaseNotes"]}, {"en-US", "fr-FR", "de-DE"}) - self.assertEqual(dict(s.puts)["alpha"]["releases"], []) + self.assertEqual( + dict(s.puts)["alpha"]["releases"], + [{"name": "3.49.14.86", "versionCodes": ["86"], "status": "halted"}], + ) self.assertEqual(len(s.commits), 1) def test_the_committed_edit_is_never_deleted(self): @@ -165,15 +172,28 @@ def test_a_failure_after_the_commit_leaves_the_committed_edit_alone(self): self.assertEqual(len(s.commits), 1) self.assertEqual(s.deleted, []) # deleting a committed edit is an API error - def test_a_body_less_200_on_the_clear_is_not_an_error(self): - s = FakeSession() - run(promote("--clear-track", "alpha"), s) + def test_a_body_less_200_is_not_an_error(self): + s = FakeSession(body_less_put=True) + run(promote("--halt-track", "alpha"), s) self.assertEqual([t for t, _ in s.puts], ["beta", "alpha"]) self.assertEqual(len(s.commits), 1) + def test_halting_a_track_with_no_release_aborts(self): + s = FakeSession() + with self.assertRaises(SystemExit): + run(promote("--halt-track", "empty"), s) + + def test_halting_an_already_halted_track_aborts(self): + s = FakeSession() + halted = [{"name": "x", "versionCodes": ["86"], "status": "halted"}] + with mock.patch.dict(TRACKS["tracks"][1], {"releases": halted}): + with self.assertRaises(SystemExit): + run(promote("--halt-track", "alpha"), s) + self.assertEqual(s.puts, []) + def test_dry_run_commits_nothing(self): s = FakeSession() - out = run(promote("--clear-track", "alpha", "--dry-run"), s) + out = run(promote("--halt-track", "alpha", "--dry-run"), s) self.assertEqual(s.puts, []) self.assertEqual(s.commits, []) self.assertIn("dry run", out) @@ -207,7 +227,7 @@ def test_an_error_naming_no_locale_in_the_body_aborts(self): run(promote(), s) self.assertEqual(s.puts, [], message) - def test_clearing_the_promote_target_aborts(self): + def test_halting_the_promote_target_aborts(self): s = FakeSession() with self.assertRaises(SystemExit): run( @@ -218,7 +238,7 @@ def test_clearing_the_promote_target_aborts(self): "alpha", "--notes-dir", notes_dir(), - "--clear-track", + "--halt-track", "alpha", ], s, @@ -226,10 +246,10 @@ def test_clearing_the_promote_target_aborts(self): self.assertEqual(s.puts, []) self.assertEqual(s.commits, []) - def test_clearing_production_aborts(self): + def test_halting_production_aborts(self): s = FakeSession() with self.assertRaises(SystemExit): - run(promote("--clear-track", "production"), s) + run(promote("--halt-track", "production"), s) self.assertEqual(s.puts, []) def test_multi_code_source_release_aborts(self):