From 1a27fa34891b4c8e1b6b2c5592041d85a1539888 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Tue, 7 Nov 2023 11:03:44 +0100 Subject: [PATCH] Fix fragment parsing with stray numbers and dots fixes #562 --- .gitignore | 1 - src/towncrier/_builder.py | 14 +++----- src/towncrier/newsfragments/562.bugfix | 1 + src/towncrier/newsfragments/562.removal | 1 + src/towncrier/test/test_build.py | 9 +++-- src/towncrier/test/test_builder.py | 48 +++++++++++++++++++++++-- 6 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 src/towncrier/newsfragments/562.bugfix create mode 100644 src/towncrier/newsfragments/562.removal diff --git a/.gitignore b/.gitignore index b06a90d8..e101cbb7 100644 --- a/.gitignore +++ b/.gitignore @@ -18,7 +18,6 @@ .vscode Justfile *egg-info/ -towncrier.test.test_*/ _trial_temp*/ apidocs/ dist/ diff --git a/src/towncrier/_builder.py b/src/towncrier/_builder.py index 3f72f1fa..3d863548 100644 --- a/src/towncrier/_builder.py +++ b/src/towncrier/_builder.py @@ -32,24 +32,20 @@ def parse_newfragment_basename( if len(parts) == 1: return invalid - if len(parts) == 2: - ticket, category = parts - ticket = strip_if_integer_string(ticket) - return (ticket, category, 0) if category in frag_type_names else invalid - # There are at least 3 parts. Search for a valid category from the second - # part onwards. + # There are at least 2 parts. Search for a valid category from the second + # part onwards starting at the back. # The category is used as the reference point in the parts list to later # infer the issue number and counter value. - for i in range(1, len(parts)): + for i in reversed(range(1, len(parts))): if parts[i] in frag_type_names: # Current part is a valid category according to given definitions. category = parts[i] - # Use the previous part as the ticket number. + # Use all previous parts as the ticket number. # NOTE: This allows news fragment names like fix-1.2.3.feature or # something-cool.feature.ext for projects that don't use ticket # numbers in news fragment names. - ticket = strip_if_integer_string(parts[i - 1]) + ticket = strip_if_integer_string(".".join(parts[0:i])) counter = 0 # Use the following part as the counter if it exists and is a valid # digit. diff --git a/src/towncrier/newsfragments/562.bugfix b/src/towncrier/newsfragments/562.bugfix new file mode 100644 index 00000000..5fb442b6 --- /dev/null +++ b/src/towncrier/newsfragments/562.bugfix @@ -0,0 +1 @@ +Parsing news fragments is now more robust in the face of non numeric fragment names containing numbers and dots. diff --git a/src/towncrier/newsfragments/562.removal b/src/towncrier/newsfragments/562.removal new file mode 100644 index 00000000..a65f502f --- /dev/null +++ b/src/towncrier/newsfragments/562.removal @@ -0,0 +1 @@ +Previously, fragment filenames like ``fix-1.2.3.feature`` identified as closing issue ``3``. Now the identifier of newsfragment is presented as ``fix-1.2.3``. diff --git a/src/towncrier/test/test_build.py b/src/towncrier/test/test_build.py index 52f82829..938814cc 100644 --- a/src/towncrier/test/test_build.py +++ b/src/towncrier/test/test_build.py @@ -60,7 +60,7 @@ def _test_command(self, command): result = runner.invoke(command, ["--draft", "--date", "01-01-2001"]) - self.assertEqual(0, result.exit_code) + self.assertEqual(0, result.exit_code, result.output) self.assertEqual( result.output, dedent( @@ -78,7 +78,7 @@ def _test_command(self, command): -------- - Baz levitation (baz) - - Baz fix levitation (#2) + - Baz fix levitation (fix-1.2) - Adds levitation (#123) - Extends levitation (#124) - An orphaned feature ending with a dotted number @@ -417,6 +417,7 @@ def test_draft_no_date(self): call(["git", "init"]) call(["git", "config", "user.name", "user"]) call(["git", "config", "user.email", "user@example.com"]) + call(["git", "config", "commit.gpgSign", "false"]) call(["git", "add", "."]) call(["git", "commit", "-m", "Initial Commit"]) @@ -441,6 +442,7 @@ def test_no_confirmation(self): call(["git", "init"]) call(["git", "config", "user.name", "user"]) call(["git", "config", "user.email", "user@example.com"]) + call(["git", "config", "commit.gpgSign", "false"]) call(["git", "add", "."]) call(["git", "commit", "-m", "Initial Commit"]) @@ -470,6 +472,7 @@ def test_keep_fragments(self, runner): call(["git", "init"]) call(["git", "config", "user.name", "user"]) call(["git", "config", "user.email", "user@example.com"]) + call(["git", "config", "commit.gpgSign", "false"]) call(["git", "add", "."]) call(["git", "commit", "-m", "Initial Commit"]) @@ -503,6 +506,7 @@ def test_yes_keep_error(self, runner): call(["git", "init"]) call(["git", "config", "user.name", "user"]) call(["git", "config", "user.email", "user@example.com"]) + call(["git", "config", "commit.gpgSign", "false"]) call(["git", "add", "."]) call(["git", "commit", "-m", "Initial Commit"]) @@ -531,6 +535,7 @@ def test_confirmation_says_no(self): call(["git", "init"]) call(["git", "config", "user.name", "user"]) call(["git", "config", "user.email", "user@example.com"]) + call(["git", "config", "commit.gpgSign", "false"]) call(["git", "add", "."]) call(["git", "commit", "-m", "Initial Commit"]) diff --git a/src/towncrier/test/test_builder.py b/src/towncrier/test/test_builder.py index 62630af0..b62033ac 100644 --- a/src/towncrier/test/test_builder.py +++ b/src/towncrier/test/test_builder.py @@ -8,63 +8,73 @@ class TestParseNewsfragmentBasename(TestCase): def test_simple(self): + """. generates a counter value of 0.""" self.assertEqual( parse_newfragment_basename("123.feature", ["feature"]), ("123", "feature", 0), ) def test_invalid_category(self): + """Files without a valid category are rejected.""" self.assertEqual( parse_newfragment_basename("README.ext", ["feature"]), (None, None, None), ) def test_counter(self): + """.. generates a custom counter value.""" self.assertEqual( parse_newfragment_basename("123.feature.1", ["feature"]), ("123", "feature", 1), ) def test_counter_with_extension(self): + """File extensions are ignored.""" self.assertEqual( parse_newfragment_basename("123.feature.1.ext", ["feature"]), ("123", "feature", 1), ) def test_ignores_extension(self): + """File extensions are ignored.""" self.assertEqual( parse_newfragment_basename("123.feature.ext", ["feature"]), ("123", "feature", 0), ) def test_non_numeric_ticket(self): + """Non-numeric issue identifiers are preserved verbatim.""" self.assertEqual( parse_newfragment_basename("baz.feature", ["feature"]), ("baz", "feature", 0), ) def test_non_numeric_ticket_with_extension(self): + """File extensions are ignored.""" self.assertEqual( parse_newfragment_basename("baz.feature.ext", ["feature"]), ("baz", "feature", 0), ) def test_dots_in_ticket_name(self): + """Non-numeric issue identifiers are preserved verbatim.""" self.assertEqual( parse_newfragment_basename("baz.1.2.feature", ["feature"]), - ("2", "feature", 0), + ("baz.1.2", "feature", 0), ) def test_dots_in_ticket_name_invalid_category(self): + """Files without a valid category are rejected.""" self.assertEqual( parse_newfragment_basename("baz.1.2.notfeature", ["feature"]), (None, None, None), ) def test_dots_in_ticket_name_and_counter(self): + """Non-numeric issue identifiers are preserved verbatim.""" self.assertEqual( parse_newfragment_basename("baz.1.2.feature.3", ["feature"]), - ("2", "feature", 3), + ("baz.1.2", "feature", 3), ) def test_strip(self): @@ -77,7 +87,41 @@ def test_strip(self): ) def test_strip_with_counter(self): + """Leading spaces and subsequent leading zeros are stripped + when parsing newsfragment names into ticket numbers etc. + """ self.assertEqual( parse_newfragment_basename(" 007.feature.3", ["feature"]), ("7", "feature", 3), ) + + def test_orphan(self): + """Orphaned snippets must remain the orphan marker in the issue + identifier.""" + self.assertEqual( + parse_newfragment_basename("+orphan.feature", ["feature"]), + ("+orphan", "feature", 0), + ) + + def test_orphan_with_number(self): + """Orphaned snippets can contain numbers in the identifier.""" + self.assertEqual( + parse_newfragment_basename("+123_orphan.feature", ["feature"]), + ("+123_orphan", "feature", 0), + ) + self.assertEqual( + parse_newfragment_basename("+orphan_123.feature", ["feature"]), + ("+orphan_123", "feature", 0), + ) + + def test_orphan_with_dotted_number(self): + """Orphaned snippets can contain numbers with dots in the + identifier.""" + self.assertEqual( + parse_newfragment_basename("+12.3_orphan.feature", ["feature"]), + ("+12.3_orphan", "feature", 0), + ) + self.assertEqual( + parse_newfragment_basename("+orphan_12.3.feature", ["feature"]), + ("+orphan_12.3", "feature", 0), + )