diff --git a/build/generate-changes.py b/build/generate-changes.py index 9fefd54838a..ac76948c0fb 100755 --- a/build/generate-changes.py +++ b/build/generate-changes.py @@ -24,14 +24,18 @@ Three complementary data sources are combined: 1. JIRA (primary, when --jira-version is given) - Queries all tickets where fixVersion = VERSION and status is resolved. + Queries all tickets where fixVersion = VERSION and status is resolved, + except those resolved without a change (Won't Fix, Not A Problem, ...). This is the authoritative list used in actual releases. 2. Git commits (always) Walks commits between the last v* tag (or --from) and the branch tip. Extracts THRIFT-NNNN references from commit messages and fetches their - JIRA summaries. Commits with no ticket reference are included as - GitHub commit links, grouped by their "Client:" trailer. + JIRA summaries. A referenced ticket is listed only if JIRA files it + under the release version the same way the fixVersion query does, so a + commit that merely mentions an older ticket does not list it. Commits + with no such ticket are included as GitHub commit links, grouped by + their "Client:" trailer. 3. GitHub PR labels (fallback for commits without a "Client:" trailer) When a commit was merged via a PR (subject ends with "(#NNN)") and has @@ -41,8 +45,10 @@ With --github-token the script also resolves PR numbers for commits whose subject lacks the "(#NNN)" suffix, so all commit links point to their PR. -When --jira-version is NOT given the script is git-only (useful while a -release is still in progress and fixVersions haven't been assigned in JIRA). +When --jira-version is NOT given the script is git-only: tickets come from +commit messages alone, filtered by the version from --version or +configure.ac. A commit whose ticket has no fixVersion assigned yet is still +listed, as a commit link. Usage: generate-changes.py [options] @@ -50,7 +56,9 @@ Options: --branch BRANCH Branch to analyze (default: current branch or master) --from TAG Starting tag or commit ref (default: auto-detect latest v* tag) - --version VERSION Release version for the ## header (default: from configure.ac) + --version VERSION Release version for the ## header and, without + --jira-version, the fixVersion a referenced ticket + needs to be listed (default: from configure.ac) --jira-version VERSION Also query JIRA for all tickets with this fixVersion; overrides git-extracted tickets as the primary source --no-commits Exclude ticket-less commits from output (default: include them) @@ -215,7 +223,9 @@ # Add any section names you want pinned to the bottom here. LATE_SECTIONS = {"(All Languages)", "(No Section)"} -TICKET_RE = re.compile(r'\bTHRIFT-(\d+)\b', re.IGNORECASE) +# A number followed by ".digit" belongs to a version, as in the path +# "thrift-0.24.0/lib/...", and is not a ticket. +TICKET_RE = re.compile(r'\bTHRIFT-(\d+)\b(?!\.\d)', re.IGNORECASE) CLIENT_TRAILER_RE = re.compile(r'\bClient:\s*(.+)', re.IGNORECASE) PR_RE = re.compile(r'\(#(\d+)\)\s*$') @@ -291,6 +301,11 @@ def extract_tickets(subject, body): return {f"THRIFT-{m.group(1)}" for m in TICKET_RE.finditer(text)} +def ticket_number(ticket_id): + """Sort key for 'THRIFT-NNNN' strings: the number NNNN.""" + return int(ticket_id.rsplit("-", 1)[1]) + + def extract_client_sections(subject, body): """Return list of canonical section names from the Client: trailer.""" text = f"{subject}\n{body}" @@ -316,6 +331,9 @@ def clean_subject(subject): subject = re.sub(r'^No\s+ticket:\s*', '', subject, flags=re.IGNORECASE) # Remove trailing "Client: ..." trailer that appears on the subject line subject = re.sub(r'\s+Client:\s*\S.*$', '', subject, flags=re.IGNORECASE) + # ... and the contributor trailers extract_client_sections() strips, also + # when they precede the Client: trailer + subject = re.sub(r'\s+(?:Patch|Autor):.*$', '', subject) # codespell:ignore # Strip trailing PR reference " (#NNN)" subject = re.sub(r'\s+\(#\d+\)\s*$', '', subject) return subject.strip() @@ -336,17 +354,65 @@ def jira_component_to_section(comp_name): return JIRA_COMPONENT_MAP.get(base, base) +# Issue fields both JIRA queries request; jira_issue_entry() reads them. +JIRA_FIELDS = "summary,components,fixVersions,status,resolution" + +# Resolutions that close a ticket without a change to report. Such tickets +# stay out of the release notes even when they carry the fix version. +NON_FIX_RESOLUTIONS = ( + "Won't Do", + "Won't Fix", + "Not A Problem", + "Not A Bug", + "Cannot Reproduce", + "Works for Me", + "Invalid", + "Incomplete", + "Information Provided", + "Later", + "Abandoned", + "Auto Closed", +) + + +def jira_issue_entry(fields): + """Turn a JIRA issue's "fields" object into the entry the fetch helpers + return: + {"summary": str, "sections": [str], "fix_versions": [str], + "status": str, "resolution": str or None} + """ + raw_sections = [ + jira_component_to_section(c["name"]) + for c in fields.get("components") or [] + ] + # Deduplicate, preserving order + seen: set = set() + sections = [] + for s in raw_sections: + if s not in seen: + seen.add(s) + sections.append(s) + return { + "summary": fields["summary"], + "sections": sections if sections else ["(No Section)"], + "fix_versions": [v["name"] for v in fields.get("fixVersions") or []], + "status": (fields.get("status") or {}).get("name"), + "resolution": (fields.get("resolution") or {}).get("name"), + } + + def fetch_jira_issues(ticket_ids): - """Query JIRA for summary + components. + """Query JIRA for the given tickets. - Returns dict mapping ticket_id (uppercase) to - {"summary": str, "sections": [str]} - Unknown / unreachable tickets are absent from the result. + Returns dict mapping ticket_id (uppercase) to a jira_issue_entry(). + Unknown / unreachable tickets are absent from the result; the unknown + ones are reported on stderr. """ if not ticket_ids: return {} result = {} + unknown = [] ticket_list = sorted(ticket_ids) for i in range(0, len(ticket_list), 50): @@ -354,8 +420,13 @@ def fetch_jira_issues(ticket_ids): keys = ",".join(batch) params = urlencode({ "jql": f"key in ({keys})", - "fields": "summary,components", + "fields": JIRA_FIELDS, "maxResults": 50, + # A validating JIRA rejects a list of up to 25 keys as a whole if + # one of them does not exist, losing all the others. Only keys + # are looked up here, so skip the check and report the keys that + # come back empty instead. + "validateQuery": "false", }) url = f"{JIRA_BASE}/rest/api/2/search?{params}" try: @@ -363,35 +434,27 @@ def fetch_jira_issues(ticket_ids): with urllib.request.urlopen(req, timeout=30) as resp: data = json.loads(resp.read()) for issue in data.get("issues", []): - key = issue["key"].upper() - summary = issue["fields"]["summary"] - raw_sections = [ - jira_component_to_section(c["name"]) - for c in issue["fields"].get("components", []) - ] - # Deduplicate, preserving order - seen: set = set() - sections = [] - for s in raw_sections: - if s not in seen: - seen.add(s) - sections.append(s) - result[key] = { - "summary": summary, - "sections": sections if sections else ["(No Section)"], - } + result[issue["key"].upper()] = jira_issue_entry(issue["fields"]) + unknown.extend(k for k in batch if k.upper() not in result) except (urllib.error.URLError, urllib.error.HTTPError, json.JSONDecodeError) as exc: print(f"Warning: JIRA query failed: {exc}", file=sys.stderr) if i + 50 < len(ticket_list): time.sleep(0.3) + if unknown: + unknown.sort(key=ticket_number) + print( + f"Warning: referenced tickets not found in JIRA: {', '.join(unknown)}", + file=sys.stderr, + ) return result def fetch_jira_by_fixversion(fix_version): """Return the same dict format as fetch_jira_issues, for all tickets that - have fixVersion = fix_version and are resolved/closed. + have fixVersion = fix_version and are resolved/closed, except those with + a resolution in NON_FIX_RESOLUTIONS. This is the authoritative JIRA query described in ReleaseManagement.md: project = THRIFT AND resolution = Fixed @@ -401,18 +464,22 @@ def fetch_jira_by_fixversion(fix_version): start_at = 0 page_size = 100 - # Include any resolved/closed ticket regardless of resolution sub-type. - # The release manager occasionally assigns fixVersion to Duplicate or - # similar tickets when they were addressed as part of the release. + # Include resolved/closed tickets beyond resolution = Fixed: the release + # manager occasionally assigns fixVersion to Duplicate or similar tickets + # when they were addressed as part of the release. Resolutions that + # record no change (NON_FIX_RESOLUTIONS) stay out. + # jira_ticket_in_release() applies the same test to tickets found in git. + non_fix = ", ".join(f'"{name}"' for name in NON_FIX_RESOLUTIONS) jql = ( f'project = THRIFT AND resolution != Unresolved ' + f'AND resolution not in ({non_fix}) ' f'AND fixVersion = "{fix_version}" AND status != Open' ) while True: params = urlencode({ "jql": jql, - "fields": "summary,components", + "fields": JIRA_FIELDS, "maxResults": page_size, "startAt": start_at, }) @@ -426,22 +493,7 @@ def fetch_jira_by_fixversion(fix_version): break for issue in data.get("issues", []): - key = issue["key"].upper() - summary = issue["fields"]["summary"] - raw_sections = [ - jira_component_to_section(c["name"]) - for c in issue["fields"].get("components", []) - ] - seen: set = set() - sections = [] - for s in raw_sections: - if s not in seen: - seen.add(s) - sections.append(s) - result[key] = { - "summary": summary, - "sections": sections if sections else ["(No Section)"], - } + result[issue["key"].upper()] = jira_issue_entry(issue["fields"]) total = data.get("total", 0) start_at += page_size @@ -452,6 +504,51 @@ def fetch_jira_by_fixversion(fix_version): return result +def jira_ticket_in_release(entry, version): + """True if JIRA files the ticket under version the way the fixVersion + query in fetch_jira_by_fixversion() does: version is among its fix + versions, it has a resolution that is not in NON_FIX_RESOLUTIONS, and + it is not Open.""" + return ( + version in entry["fix_versions"] and + entry["resolution"] is not None and + entry["resolution"] not in NON_FIX_RESOLUTIONS and + entry["status"] != "Open" + ) + + +def filter_release_tickets(jira_data, version): + """Split fetched JIRA entries by jira_ticket_in_release(). + + Returns (kept, skipped): the entries in the release, and the keys of the + others in ticket-number order.""" + kept = {} + skipped = [] + for key, entry in jira_data.items(): + if jira_ticket_in_release(entry, version): + kept[key] = entry + else: + skipped.append(key) + skipped.sort(key=ticket_number) + return kept, skipped + + +def fetch_release_tickets(ticket_ids, version): + """fetch_jira_issues() restricted to the tickets in the release. + + A commit message may name a ticket that belongs to another release, for + instance as history. Such tickets are reported and left out, so commits + that reference nothing else are listed as commit links instead.""" + kept, skipped = filter_release_tickets(fetch_jira_issues(ticket_ids), version) + if skipped: + print( + f"Skipping {len(skipped)} referenced tickets that JIRA does not " + f"list as fixed in {version}: {', '.join(skipped)}", + file=sys.stderr, + ) + return kept + + # --------------------------------------------------------------------------- # GitHub helpers # --------------------------------------------------------------------------- @@ -662,6 +759,8 @@ def generate_changes(args): # --- Version --- version = args.version or version_from_configure(repo_root) or "X.Y.Z" print(f"Version : {version}", file=sys.stderr) + # The fixVersion a ticket referenced in git must carry to be listed. + release_version = args.jira_version or version # --- Commits --- commits = get_commits(since, branch, repo_root) @@ -700,11 +799,11 @@ def generate_changes(args): f"Fetching {len(extra)} additional git-referenced tickets from JIRA ...", file=sys.stderr, ) - jira_data.update(fetch_jira_issues(extra)) + jira_data.update(fetch_release_tickets(extra, release_version)) else: if all_tickets: print(f"Querying JIRA for {len(all_tickets)} git-referenced tickets ...", file=sys.stderr) - jira_data = fetch_jira_issues(all_tickets) + jira_data = fetch_release_tickets(all_tickets, release_version) print(f"Total JIRA entries: {len(jira_data)}", file=sys.stderr) @@ -859,14 +958,19 @@ def main(): ) parser.add_argument( "--version", metavar="VERSION", - help="release version for the ## header (default: read from configure.ac)", + help=( + "release version for the ## header and, without --jira-version, " + "the fixVersion a git-referenced ticket needs to be listed " + "(default: read from configure.ac)" + ), ) parser.add_argument( "--jira-version", dest="jira_version", metavar="VERSION", help=( "query JIRA for all tickets with this fixVersion as the primary " "source (recommended for release prep once fixVersions are assigned); " - "git-extracted tickets are merged in as a supplement" + "git-extracted tickets with the same fixVersion are merged in as a " + "supplement" ), ) parser.add_argument( diff --git a/build/test_generate_changes.py b/build/test_generate_changes.py index f335f9981b7..d36e1c8617b 100644 --- a/build/test_generate_changes.py +++ b/build/test_generate_changes.py @@ -18,7 +18,7 @@ # under the License. # -"""Unit tests for build/generate-changes.py section-assignment logic. +"""Unit tests for build/generate-changes.py. These cover the three cases that previously produced "(No Section)" entries: @@ -29,13 +29,28 @@ 3. JIRA tickets with no usable component, filed under the section named by their commit's Client: trailer instead (JIRA -> trailer fallback). -No network access is required: only the pure mapping/assignment helpers are -exercised. +They also cover which referenced tickets get a JIRA line: only those JIRA +files under the release version. A commit that merely mentions an old +ticket (THRIFT-6183 citing THRIFT-1337) must not list it. Those tests run +generate_changes() against a temporary git repository and a fake JIRA. + +No network access is required. """ +import argparse +import contextlib import importlib.util +import io +import json import os +import re +import shutil +import subprocess +import tempfile import unittest +import urllib.error +import urllib.parse +from unittest import mock # generate-changes.py has a hyphen in its name, so it cannot be imported with a # plain ``import``; load it as a module from its path instead. @@ -59,6 +74,108 @@ def make_commit(sha="0" * 40, pr_num=None, sections=None, tickets=()): } +def jira_fields(summary, components=(), fix_versions=(), status="Resolved", + resolution="Fixed"): + """Build an issue's "fields" object as the JIRA REST API returns it.""" + return { + "summary": summary, + "components": [{"name": c} for c in components], + "fixVersions": [{"name": v} for v in fix_versions], + "status": {"name": status}, + "resolution": {"name": resolution} if resolution else None, + } + + +class FakeJira: + """Stands in for urllib.request.urlopen and answers the two JIRA searches + generate-changes.py sends ("key in (...)" and the fixVersion query) from + a {key: fields} table. Like the real REST API it returns only the + fields the request asked for, and it rejects a key list that names an + unknown key when it validates the query. Any other request fails the + test.""" + + # issues.apache.org checks that each key exists only in key lists up to + # this length, and only while validateQuery is on (the default). + VALIDATED_KEY_LIST = 25 + + def __init__(self, issues): + self.issues = issues + + @staticmethod + def matches_fix_version_query(jql, fields, version): + # resolution != Unresolved [AND resolution not in (...)] + # AND fixVersion = version AND status != Open + excluded = re.search(r"resolution not in \(([^)]*)\)", jql) + names = ( + [n.strip().strip('"') for n in excluded.group(1).split(",")] + if excluded else [] + ) + resolution = fields["resolution"] + return ( + resolution is not None and + resolution["name"] not in names and + version in [v["name"] for v in fields["fixVersions"]] and + fields["status"]["name"] != "Open" + ) + + def urlopen(self, req, timeout=None): + prefix = f"{gc.JIRA_BASE}/rest/api/2/search?" + url = req.full_url + if not url.startswith(prefix): + raise AssertionError(f"unexpected request: {url}") + query = urllib.parse.parse_qs(url[len(prefix):]) + jql = query["jql"][0] + wanted = query["fields"][0].split(",") + start = int(query.get("startAt", ["0"])[0]) + keys = re.fullmatch(r"key in \((.*)\)", jql) + if keys: + requested = keys.group(1).split(",") + unknown = [k for k in requested if k not in self.issues] + validate = query.get("validateQuery", ["true"])[0].lower() == "true" + if validate and unknown and len(requested) <= self.VALIDATED_KEY_LIST: + error = {"errorMessages": [ + f"An issue with key '{k}' does not exist for field 'key'." + for k in unknown + ], "errors": {}} + raise urllib.error.HTTPError( + url, 400, "Bad Request", None, + io.BytesIO(json.dumps(error).encode("utf-8")), + ) + hits = [k for k in requested if k in self.issues] + else: + version = re.search(r'fixVersion = "([^"]+)"', jql).group(1) + hits = [ + k for k, f in self.issues.items() + if self.matches_fix_version_query(jql, f, version) + ] + body = { + "total": len(hits), + "issues": [ + { + "key": k, + "fields": { + name: self.issues[k][name] + for name in wanted if name in self.issues[k] + }, + } + for k in hits[start:] + ], + } + return io.BytesIO(json.dumps(body).encode("utf-8")) + + +def sections_of(draft): + """Map each ### heading of a rendered draft to its bullet lines.""" + result = {} + bullets = None + for line in draft.splitlines(): + if line.startswith("### "): + bullets = result.setdefault(line[len("### "):], []) + elif line.startswith("- ") and bullets is not None: + bullets.append(line) + return result + + class LabelMappingTests(unittest.TestCase): """Fix 1: dependabot / CI labels route to the Build Process section.""" @@ -212,5 +329,313 @@ def test_jira_components_map_to_zig(self): self.assertEqual(gc.jira_component_to_section("Zig - Compiler"), "Zig") +class TicketExtractionTests(unittest.TestCase): + """Which THRIFT-NNNN mentions in a commit message count as tickets.""" + + def test_version_string_is_not_a_ticket(self): + # Mirrors da6ed655d, whose body quotes a path in the 0.24.0 tarball. + self.assertEqual(gc.extract_tickets( + "Add cstddef include to fix build error with 6.3.0", + "thrift-0.24.0/lib/cpp/src/thrift/transport/TBufferTransports.h:110:32:", + ), set()) + + def test_ticket_at_the_end_of_a_sentence_is_kept(self): + self.assertEqual( + gc.extract_tickets("Fix the frame size", "Follows up on THRIFT-1337."), + {"THRIFT-1337"}, + ) + + def test_ticket_prefix_in_any_case_is_kept(self): + self.assertEqual( + gc.extract_tickets("Thrift-2600: 0.9.2 release", ""), + {"THRIFT-2600"}, + ) + + +class CleanSubjectTests(unittest.TestCase): + """A commit line shows the subject without its ticket and trailers.""" + + def test_patch_trailer_before_client_trailer_is_stripped(self): + # Mirrors 2ae9c11db, listed by commit while THRIFT-6108 is open. + self.assertEqual( + gc.clean_subject( + "THRIFT-6108: Consolidate replace_all() into t_oop_generator" + " Patch: A. Contributor Client: dart,delphi" + ), + "Consolidate replace_all() into t_oop_generator", + ) + + def test_autor_trailer_is_stripped(self): + self.assertEqual( + gc.clean_subject("THRIFT-1: Fix the build Autor: A. Contributor"), # codespell:ignore + "Fix the build", + ) + + +TICKET_6183 = jira_fields( + "Use the library-wide default frame size in TNonblockingServer", + components=["C++ - Library"], fix_versions=["0.25.0"], +) + +# Fixed in 2011 and never given a Fix Version/s. +TICKET_1337 = jira_fields( + "thrift: support maximum frame size in TNonblockingServer", + components=["C++ - Library"], status="Closed", +) + + +class ReleaseTicketFilterTests(unittest.TestCase): + """A ticket is in the release only if JIRA files it under that version.""" + + def in_release(self, version="0.25.0", **fields): + entry = gc.jira_issue_entry(jira_fields("summary", **fields)) + return gc.jira_ticket_in_release(entry, version) + + def test_resolved_ticket_with_the_release_fix_version_is_in(self): + self.assertTrue(self.in_release(fix_versions=["0.25.0"])) + + def test_ticket_without_a_fix_version_is_out(self): + self.assertFalse(self.in_release(status="Closed")) + + def test_ticket_fixed_in_another_version_is_out(self): + self.assertFalse(self.in_release(fix_versions=["0.24.0"])) + + def test_release_among_several_fix_versions_is_in(self): + self.assertTrue(self.in_release(fix_versions=["0.24.1", "0.25.0"])) + + def test_open_ticket_is_out_even_with_the_release_fix_version(self): + self.assertFalse(self.in_release( + fix_versions=["0.25.0"], status="Open", resolution=None + )) + + def test_unresolved_ticket_is_out_even_when_not_open(self): + self.assertFalse(self.in_release( + fix_versions=["0.25.0"], status="In Progress", resolution=None + )) + + def test_resolutions_that_record_a_change_are_in(self): + # Duplicate included: the release manager sometimes gives a duplicate + # the fix version when the release addressed it. + for resolution in ["Fixed", "Done", "Implemented", "Duplicate"]: + with self.subTest(resolution=resolution): + self.assertTrue(self.in_release( + fix_versions=["0.25.0"], status="Closed", + resolution=resolution, + )) + + def test_resolutions_without_a_fix_are_out(self): + # THRIFT-5917 (Won't Do): not a change to report, whatever its + # Fix Version/s says. + for resolution in [ + "Won't Do", "Won't Fix", "Not A Problem", "Not A Bug", + "Cannot Reproduce", "Works for Me", "Invalid", "Incomplete", + "Information Provided", "Later", "Abandoned", "Auto Closed", + ]: + with self.subTest(resolution=resolution): + self.assertFalse(self.in_release( + fix_versions=["0.25.0"], status="Closed", + resolution=resolution, + )) + + def test_entry_keeps_the_fields_the_filter_needs(self): + entry = gc.jira_issue_entry(jira_fields( + "summary", + components=["C++ - Library", "C++ - Compiler"], + fix_versions=["0.25.0"], + )) + self.assertEqual(entry, { + "summary": "summary", + "sections": ["C++"], + "fix_versions": ["0.25.0"], + "status": "Resolved", + "resolution": "Fixed", + }) + + def test_filter_reports_skipped_tickets_in_ticket_order(self): + data = { + "THRIFT-6183": gc.jira_issue_entry(TICKET_6183), + "THRIFT-1337": gc.jira_issue_entry(TICKET_1337), + "THRIFT-892": gc.jira_issue_entry( + jira_fields("summary", fix_versions=["0.7"]) + ), + } + kept, skipped = gc.filter_release_tickets(data, "0.25.0") + self.assertEqual(list(kept), ["THRIFT-6183"]) + self.assertEqual(skipped, ["THRIFT-892", "THRIFT-1337"]) + + +@unittest.skipUnless(shutil.which("git"), "git is not installed") +class ReleaseTicketDraftTests(unittest.TestCase): + """End to end: which referenced tickets the rendered draft lists.""" + + CPP_6183 = ( + "- [THRIFT-6183](https://issues.apache.org/jira/browse/THRIFT-6183)" + " - Use the library-wide default frame size in TNonblockingServer" + ) + + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.repo = os.path.join(tmp.name, "repo") + self.output = os.path.join(tmp.name, "CHANGES-draft.md") + os.mkdir(self.repo) + # Keep the user's git configuration away from both the setup below and + # the script's own git calls. + env = mock.patch.dict(os.environ, { + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@example.org", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@example.org", + }) + env.start() + self.addCleanup(env.stop) + self.git("init", "-q") + configure_ac = os.path.join(self.repo, "configure.ac") + with open(configure_ac, "w", encoding="utf-8") as f: + f.write("AC_INIT([thrift], [0.25.0], [dev@thrift.apache.org])\n") + self.git("add", "configure.ac") + self.git("commit", "-q", "-m", "Set the version to 0.25.0") + self.git("tag", "v0.24.0") + + def git(self, *args): + subprocess.run( + ["git"] + list(args), cwd=self.repo, check=True, capture_output=True + ) + + def commit(self, message): + self.git("commit", "-q", "--allow-empty", "-m", message) + + def generate(self, issues, jira_version=None): + """Run generate_changes() on the repository; return (draft, stderr).""" + args = argparse.Namespace( + branch=None, from_tag=None, version=None, jira_version=jira_version, + no_commits=False, github_token=None, repo="apache/thrift", + output=self.output, + ) + log = io.StringIO() + fake = FakeJira(issues) + with mock.patch.object(gc, "find_repo_root", return_value=self.repo), \ + mock.patch.object(gc.urllib.request, "urlopen", fake.urlopen), \ + contextlib.redirect_stderr(log): + gc.generate_changes(args) + with open(self.output, encoding="utf-8") as f: + return f.read(), log.getvalue() + + def commit_6183(self): + # Mirrors 8b79397f7, whose body names THRIFT-1337 only as history. + self.commit( + "THRIFT-6183: Use the library-wide default frame size in" + " TNonblockingServer\n" + "\n" + "TNonblockingServer caps the frame it will accept at its own\n" + "MAX_FRAME_SIZE, 256 * 1024 * 1024 since THRIFT-1337 landed it in\n" + "2011.\n" + "\n" + "Client: cpp\n" + ) + + def test_ticket_mentioned_in_a_commit_body_is_not_listed(self): + self.commit_6183() + draft, log = self.generate( + {"THRIFT-6183": TICKET_6183, "THRIFT-1337": TICKET_1337} + ) + self.assertEqual(sections_of(draft), {"C++": [self.CPP_6183]}) + self.assertIn("THRIFT-1337", log) + + def test_jira_version_mode_does_not_add_it_back(self): + # The fixVersion query finds THRIFT-6183; THRIFT-1337 only comes in + # through the extra lookup of tickets the commits reference. + self.commit_6183() + draft, log = self.generate( + {"THRIFT-6183": TICKET_6183, "THRIFT-1337": TICKET_1337}, + jira_version="0.25.0", + ) + self.assertEqual(sections_of(draft), {"C++": [self.CPP_6183]}) + self.assertIn("THRIFT-1337", log) + + def test_subject_ticket_outside_the_release_is_listed_by_commit(self): + # Mirrors f62e1b4bf: THRIFT-1941 was closed without a Fix Version/s, + # so the commit is listed by its PR and subject instead. + self.commit( + "THRIFT-1941: Add PHP serializer regression coverage (#3794)\n" + "\n" + "Client: php\n" + ) + draft, _ = self.generate({ + "THRIFT-1941": jira_fields( + "PHP Serializer deserialize doesn't work", + components=["PHP - Library"], status="Closed", + ), + }) + self.assertEqual(sections_of(draft), {"PHP": [ + "- [#3794](https://github.com/apache/thrift/pull/3794)" + " - Add PHP serializer regression coverage" + ]}) + + def test_ticket_resolved_without_a_fix_is_listed_by_commit(self): + # Like THRIFT-5917, but carrying the fix version, so that the + # fixVersion query itself has to leave it out. + self.commit( + "THRIFT-5917: Remove Rust deprecation warning (#3637)\n" + "\n" + "Client: rs\n" + ) + issues = { + "THRIFT-5917": jira_fields( + "Drop Rust support?", components=["Rust - Library"], + fix_versions=["0.25.0"], status="Closed", resolution="Won't Do", + ), + } + expected = {"Rust": [ + "- [#3637](https://github.com/apache/thrift/pull/3637)" + " - Remove Rust deprecation warning" + ]} + for jira_version in [None, "0.25.0"]: + with self.subTest(jira_version=jira_version): + draft, _ = self.generate(issues, jira_version=jira_version) + self.assertEqual(sections_of(draft), expected) + + def test_version_string_in_a_commit_body_costs_no_ticket(self): + # Mirrors da6ed655d. "thrift-0.24.0/..." was read as THRIFT-0, and + # JIRA then rejected the whole lookup, THRIFT-6183 included. + self.commit_6183() + self.commit( + "Add cstddef include to fix build error with 6.3.0 (#3801)\n" + "\n" + "thrift-0.24.0/lib/cpp/src/thrift/transport/TBufferTransports.h:110:32:\n" + " error: 'ptrdiff_t' does not name a type\n" + "\n" + "Client: cpp\n" + ) + draft, log = self.generate({"THRIFT-6183": TICKET_6183}) + self.assertEqual(sections_of(draft), {"C++": [ + self.CPP_6183, + "- [#3801](https://github.com/apache/thrift/pull/3801)" + " - Add cstddef include to fix build error with 6.3.0", + ]}) + self.assertNotRegex(log, r"\bTHRIFT-0\b") + + def test_unknown_ticket_key_costs_no_other_ticket(self): + # A key that does not exist, such as a typo, must not hide the + # tickets that other commits reference. + self.commit_6183() + self.commit( + "Clarify the frame size documentation (#3900)\n" + "\n" + "Follows up on THRIFT-99999.\n" + "\n" + "Client: cpp\n" + ) + draft, log = self.generate({"THRIFT-6183": TICKET_6183}) + self.assertEqual(sections_of(draft), {"C++": [ + self.CPP_6183, + "- [#3900](https://github.com/apache/thrift/pull/3900)" + " - Clarify the frame size documentation", + ]}) + self.assertIn("THRIFT-99999", log) + + if __name__ == "__main__": unittest.main()