merge: the bug-report bot re-filed itself every run on Gitea
This commit is contained in:
@@ -6,6 +6,28 @@ is deliberate: see [Releasing](docs/releasing.md). Twelve releases were cut on
|
|||||||
live, every one of those interrupts every user. An alert people learn to ignore is
|
live, every one of those interrupts every user. An alert people learn to ignore is
|
||||||
worse than no alert, because one day it carries a security fix.
|
worse than no alert, because one day it carries a security fix.
|
||||||
|
|
||||||
|
## Unreleased
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- **The compatibility bot filed a new duplicate bug report on every Gitea run.**
|
||||||
|
`find_issue()` skipped pull requests by testing for the *presence* of the
|
||||||
|
`pull_request` key. GitHub omits that key on a plain issue; Gitea sends it as
|
||||||
|
`null`. So on Gitea every issue was discarded as a PR, the lookup always came back
|
||||||
|
empty, and the bot took the "nothing filed yet" branch and opened a fresh report
|
||||||
|
each run — **nine copies on the canonical forge**, four of them filed *after* the
|
||||||
|
commit that was meant to stop precisely this. The mirror was fine, which is why it
|
||||||
|
went unnoticed: GitHub's payload shape is the one the filter was written against.
|
||||||
|
|
||||||
|
It is the same failure the anti-spam fix was written to prevent, moved from
|
||||||
|
comments to issues, and it survived because `find_issue` was the only function in
|
||||||
|
`compat_publish.py` with no test. It now has one, per forge, and the daily cron —
|
||||||
|
which had not yet run once — no longer accumulates a report a day.
|
||||||
|
|
||||||
|
The issue list is also requested with **both** paging parameters (`per_page` for
|
||||||
|
GitHub, `limit` for Gitea). Each forge ignores the other's, and Gitea's default page
|
||||||
|
is 30, so the lookup would have started missing the report again once the pile it
|
||||||
|
was creating grew past one page.
|
||||||
|
|
||||||
## v0.7.0 — 2026-07-14
|
## v0.7.0 — 2026-07-14
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import pytest
|
|||||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "tools"))
|
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "tools"))
|
||||||
|
|
||||||
import compat # noqa: E402
|
import compat # noqa: E402
|
||||||
|
import compat_publish # noqa: E402
|
||||||
from compat import ( # noqa: E402
|
from compat import ( # noqa: E402
|
||||||
NESTED,
|
NESTED,
|
||||||
PROVIDERS,
|
PROVIDERS,
|
||||||
@@ -574,3 +575,57 @@ class TestATransientNetworkBlipDoesNotWakeAnybody:
|
|||||||
"def get_restic_config(entry, credentials):\n pass\n"
|
"def get_restic_config(entry, credentials):\n pass\n"
|
||||||
)
|
)
|
||||||
assert compat.fingerprint(self._rows(worse)) != compat.fingerprint(self._rows(broken))
|
assert compat.fingerprint(self._rows(worse)) != compat.fingerprint(self._rows(broken))
|
||||||
|
|
||||||
|
|
||||||
|
class TestTheBotFindsItsOwnIssueOnBOTHForges:
|
||||||
|
"""`find_issue` decides "have I already filed this?" -- and it ran on two forges.
|
||||||
|
|
||||||
|
It used to skip pull requests with `"pull_request" not in i`. GitHub omits that key
|
||||||
|
on a plain issue; **Gitea sends it as `null`**. So on Gitea every issue looked like
|
||||||
|
a PR, the match list was always empty, and the bot took the "nothing filed yet"
|
||||||
|
branch on EVERY run: nine duplicate copies of the same report on the canonical
|
||||||
|
forge, four of them filed after the commit that was supposed to stop exactly this.
|
||||||
|
|
||||||
|
It is the same failure the spam fix was written to prevent, moved from comments to
|
||||||
|
issues -- and it survived because `find_issue` was the one function here with no
|
||||||
|
test. So the payload shapes are pinned, per forge, by hand.
|
||||||
|
"""
|
||||||
|
|
||||||
|
TITLE = compat_publish.TITLE
|
||||||
|
|
||||||
|
def _find(self, monkeypatch, payload):
|
||||||
|
monkeypatch.setattr(compat_publish, "_call", lambda *a, **k: payload)
|
||||||
|
return compat_publish.find_issue("https://forge/api", "tok", self.TITLE)
|
||||||
|
|
||||||
|
def test_gitea_sends_pull_request_as_null_and_the_issue_is_still_found(self, monkeypatch):
|
||||||
|
found = self._find(monkeypatch, [
|
||||||
|
{"number": 7, "title": self.TITLE, "state": "open", "pull_request": None},
|
||||||
|
{"number": 1, "title": self.TITLE, "state": "open", "pull_request": None},
|
||||||
|
])
|
||||||
|
assert found is not None, (
|
||||||
|
"find_issue missed a Gitea issue, so the bot files a NEW duplicate report "
|
||||||
|
"every run -- which is how nine of them piled up"
|
||||||
|
)
|
||||||
|
assert found["number"] == 1, "lowest-numbered wins"
|
||||||
|
|
||||||
|
def test_github_omits_the_key_entirely_and_the_issue_is_still_found(self, monkeypatch):
|
||||||
|
found = self._find(monkeypatch, [
|
||||||
|
{"number": 2, "title": self.TITLE, "state": "open"},
|
||||||
|
])
|
||||||
|
assert found is not None and found["number"] == 2
|
||||||
|
|
||||||
|
def test_a_real_PR_with_the_same_title_is_still_skipped_on_both(self, monkeypatch):
|
||||||
|
# The reason the filter exists at all: both forges list PRs on /issues, and
|
||||||
|
# commenting on a PR instead of the bug report would be worse than useless.
|
||||||
|
assert self._find(monkeypatch, [
|
||||||
|
{"number": 3, "title": self.TITLE, "state": "open", # Gitea PR
|
||||||
|
"pull_request": {"merged": False}},
|
||||||
|
{"number": 4, "title": self.TITLE, "state": "open", # GitHub PR
|
||||||
|
"pull_request": {"url": "https://api.github.com/..."}},
|
||||||
|
]) is None
|
||||||
|
|
||||||
|
def test_an_unrelated_issue_is_not_mistaken_for_the_report(self, monkeypatch):
|
||||||
|
assert self._find(monkeypatch, [
|
||||||
|
{"number": 1, "title": "TypeError when create B2 backup on Electric Eel",
|
||||||
|
"state": "closed", "pull_request": None},
|
||||||
|
]) is None
|
||||||
|
|||||||
@@ -69,11 +69,18 @@ def find_issue(api, token, title):
|
|||||||
existed once (an earlier version put the ref list in the title, so the identity
|
existed once (an earlier version put the ref list in the title, so the identity
|
||||||
changed whenever that set changed), and an order-dependent pick would alternate
|
changed whenever that set changed), and an order-dependent pick would alternate
|
||||||
between them -- reopening one while commenting on the other.
|
between them -- reopening one while commenting on the other.
|
||||||
|
|
||||||
|
Both forges list PRs alongside issues, but they SAY SO DIFFERENTLY: GitHub omits
|
||||||
|
the `pull_request` key on a plain issue, Gitea sends it as `null`. Testing for the
|
||||||
|
KEY therefore discards every Gitea issue as if it were a PR -- so this returned
|
||||||
|
None on every Gitea run, and the bot filed a brand-new duplicate report each time
|
||||||
|
instead of editing the one it already had. Test the VALUE; it is the only form
|
||||||
|
that is true on both.
|
||||||
"""
|
"""
|
||||||
issues = _call(f"{api}/issues?state=all&per_page=100", token)
|
issues = _call(f"{api}/issues?state=all&per_page=100&limit=100", token)
|
||||||
mine = [
|
mine = [
|
||||||
i for i in issues
|
i for i in issues
|
||||||
if i.get("title") == title and "pull_request" not in i # GitHub lists PRs here
|
if i.get("title") == title and not i.get("pull_request")
|
||||||
]
|
]
|
||||||
return min(mine, key=lambda i: i["number"]) if mine else None
|
return min(mine, key=lambda i: i["number"]) if mine else None
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user