fix: paginate repository-label inventory for gitea_set_issue_labels (#627)
Replace single-page GET labels?limit=100 with api_get_all-backed _repo_label_id_map so later-page labels (e.g. type:feature, workflow-hardening) are not falsely rejected during full-set replacement. Also add post-mutation verification, fix related MCP/CLI label inventory call sites, and add multi-page regression tests for the #601 reconciler failure mode. Closes #627
This commit is contained in:
+29
-34
@@ -28,33 +28,31 @@ class TestLabelCreation(unittest.TestCase):
|
||||
"""Verify create-or-skip logic for the label set."""
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all")
|
||||
@patch("manage_labels.api")
|
||||
def test_skips_existing_labels(self, mock_api, _auth):
|
||||
# Simulate all labels already exist
|
||||
def test_skips_existing_labels(self, mock_api, mock_get_all, _auth):
|
||||
# Simulate all labels already exist (paginated inventory #627)
|
||||
existing = [_make_label(l["name"], i) for i, l in enumerate(manage_labels.LABELS)]
|
||||
mock_api.return_value = existing # first call is GET /labels
|
||||
mock_get_all.return_value = existing
|
||||
|
||||
# Patch sys.argv to avoid --dry
|
||||
with patch.object(sys, "argv", ["manage_labels.py"]):
|
||||
with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
|
||||
manage_labels.main()
|
||||
|
||||
# The GET call happens, but no POST calls for label creation
|
||||
get_calls = [c for c in mock_api.call_args_list if c[0][0] == "GET"]
|
||||
mock_get_all.assert_called()
|
||||
post_label_calls = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c[0][0] == "POST" and c[0][1] == "/labels"
|
||||
]
|
||||
self.assertGreaterEqual(len(get_calls), 1)
|
||||
self.assertEqual(len(post_label_calls), 0)
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all", return_value=[])
|
||||
@patch("manage_labels.api")
|
||||
def test_creates_missing_labels(self, mock_api, _auth):
|
||||
def test_creates_missing_labels(self, mock_api, _get_all, _auth):
|
||||
# Simulate no existing labels
|
||||
def side_effect(method, path, auth, payload=None):
|
||||
if method == "GET" and "/labels" in path:
|
||||
return [] # no existing labels
|
||||
if method == "POST" and path == "/labels":
|
||||
return {"id": 999, "name": payload["name"]}
|
||||
if method == "PUT":
|
||||
@@ -79,15 +77,14 @@ class TestLabelCreation(unittest.TestCase):
|
||||
class TestDryRun(unittest.TestCase):
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all", return_value=[])
|
||||
@patch("manage_labels.api")
|
||||
def test_dry_run_makes_no_writes(self, mock_api, _auth):
|
||||
mock_api.return_value = [] # no existing labels
|
||||
|
||||
def test_dry_run_makes_no_writes(self, mock_api, _get_all, _auth):
|
||||
with patch.object(sys, "argv", ["manage_labels.py", "--dry"]):
|
||||
with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
|
||||
manage_labels.main()
|
||||
|
||||
# Only the GET call should be made, no POST or PUT
|
||||
# Dry run should not call write methods via api()
|
||||
for c in mock_api.call_args_list:
|
||||
method = c[0][0]
|
||||
self.assertEqual(method, "GET",
|
||||
@@ -100,13 +97,13 @@ class TestDryRun(unittest.TestCase):
|
||||
class TestLabelMapping(unittest.TestCase):
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all")
|
||||
@patch("manage_labels.api")
|
||||
def test_applies_mapping_to_issues(self, mock_api, _auth):
|
||||
def test_applies_mapping_to_issues(self, mock_api, mock_get_all, _auth):
|
||||
existing = [_make_label(l["name"], i + 1) for i, l in enumerate(manage_labels.LABELS)]
|
||||
mock_get_all.return_value = existing
|
||||
|
||||
def side_effect(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return existing
|
||||
if method == "PUT":
|
||||
return [{"name": "applied"}]
|
||||
return None
|
||||
@@ -158,11 +155,10 @@ class TestModes(unittest.TestCase):
|
||||
return [(c[0][0], c[0][1]) for c in mock_api.call_args_list]
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all", return_value=[])
|
||||
@patch("manage_labels.api")
|
||||
def test_create_labels_only_no_mapping(self, mock_api, _auth):
|
||||
def test_create_labels_only_no_mapping(self, mock_api, _get_all, _auth):
|
||||
def se(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return [] # no existing labels
|
||||
if method == "POST" and path == "/labels":
|
||||
return {"id": 1, "name": payload["name"]}
|
||||
return None
|
||||
@@ -173,14 +169,14 @@ class TestModes(unittest.TestCase):
|
||||
self.assertFalse(any(m[0] == "PUT" for m in methods)) # no mapping applied
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all")
|
||||
@patch("manage_labels.api")
|
||||
def test_apply_mapping_only_no_label_creation(self, mock_api, _auth):
|
||||
def test_apply_mapping_only_no_label_creation(self, mock_api, mock_get_all, _auth):
|
||||
existing = [_make_label(l["name"], i + 1)
|
||||
for i, l in enumerate(manage_labels.LABELS)]
|
||||
mock_get_all.return_value = existing
|
||||
|
||||
def se(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return existing
|
||||
if method == "PUT":
|
||||
return [{"name": "applied"}]
|
||||
return None
|
||||
@@ -192,13 +188,10 @@ class TestModes(unittest.TestCase):
|
||||
self.assertEqual(len(put_calls), len(manage_labels.MAPPING))
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all", return_value=[_make_label("chore", 5)])
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_appends_to_issue(self, mock_api, _auth):
|
||||
existing = [_make_label("chore", 5)]
|
||||
|
||||
def test_add_label_appends_to_issue(self, mock_api, _get_all, _auth):
|
||||
def se(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return existing
|
||||
if method == "POST":
|
||||
return [{"name": "chore"}]
|
||||
return None
|
||||
@@ -212,19 +205,21 @@ class TestModes(unittest.TestCase):
|
||||
self.assertFalse(any(c[0][0] == "PUT" for c in mock_api.call_args_list))
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all", return_value=[])
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_unknown_makes_no_write(self, mock_api, _auth):
|
||||
mock_api.side_effect = lambda *a, **k: [] if a[0] == "GET" else None
|
||||
def test_add_label_unknown_makes_no_write(self, mock_api, _get_all, _auth):
|
||||
manage_labels.main(["--add-label", "42", "ghost"])
|
||||
# Only the GET label lookup; no POST/PUT for an undefined label.
|
||||
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list))
|
||||
# No write via api() for an undefined label.
|
||||
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list)
|
||||
or len(mock_api.call_args_list) == 0)
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api_get_all", return_value=[_make_label("chore", 5)])
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_dry_makes_no_write(self, mock_api, _auth):
|
||||
mock_api.side_effect = lambda *a, **k: [_make_label("chore", 5)] if a[0] == "GET" else None
|
||||
def test_add_label_dry_makes_no_write(self, mock_api, _get_all, _auth):
|
||||
manage_labels.main(["--dry", "--add-label", "42", "chore"])
|
||||
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list))
|
||||
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list)
|
||||
or len(mock_api.call_args_list) == 0)
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
|
||||
Reference in New Issue
Block a user