Fix orphan fragments with numbers (#564)

* Add test for orphan fragments with numbers

* Fix fragment parsing with stray numbers and dots

fixes #562

Co-authored-by: Adi Roiban <[email protected]>

.gitignore

@@ -17,6 +17,7 @@
 .vs/
 .vscode
 Justfile
+*egg-info/
 _trial_temp*/
 apidocs/
 dist/

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.

src/towncrier/newsfragments/562.bugfix

@@ -0,0 +1 @@
+Orphan newsfragments containing numeric values are no longer accidentally associated to tickets. In previous versions the orphan marker was ignored and the newsfragment was associated to a ticket having the last numerical value from the filename.

src/towncrier/newsfragments/562.bugfix.1

@@ -0,0 +1,2 @@
+Fragments with filenames like `fix-1.2.3.feature` are now associated with the ticket `fix-1.2.3`.
+In previous versions they were incorrectly associated to ticket `3`.

src/towncrier/test/test_build.py

@@ -43,6 +43,14 @@ def _test_command(self, command):
                 f.write("Orphaned feature")
             with open("foo/newsfragments/+xxx.feature", "w") as f:
                 f.write("Another orphaned feature")
+            with open("foo/newsfragments/+123_orphaned.feature", "w") as f:
+                f.write("An orphaned feature starting with a number")
+            with open("foo/newsfragments/+12.3_orphaned.feature", "w") as f:
+                f.write("An orphaned feature starting with a dotted number")
+            with open("foo/newsfragments/+orphaned_123.feature", "w") as f:
+                f.write("An orphaned feature ending with a number")
+            with open("foo/newsfragments/+orphaned_12.3.feature", "w") as f:
+                f.write("An orphaned feature ending with a dotted number")
             # Towncrier ignores files that don't have a dot
             with open("foo/newsfragments/README", "w") as f:
                 f.write("Blah blah")
@@ -52,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(
@@ -70,9 +78,13 @@ 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
+                - An orphaned feature ending with a number
+                - An orphaned feature starting with a dotted number
+                - An orphaned feature starting with a number
                 - Another orphaned feature
                 - Orphaned feature
 
@@ -405,6 +417,7 @@ def test_draft_no_date(self):
             call(["git", "init"])
             call(["git", "config", "user.name", "user"])
             call(["git", "config", "user.email", "[email protected]"])
+            call(["git", "config", "commit.gpgSign", "false"])
             call(["git", "add", "."])
             call(["git", "commit", "-m", "Initial Commit"])
 
@@ -429,6 +442,7 @@ def test_no_confirmation(self):
             call(["git", "init"])
             call(["git", "config", "user.name", "user"])
             call(["git", "config", "user.email", "[email protected]"])
+            call(["git", "config", "commit.gpgSign", "false"])
             call(["git", "add", "."])
             call(["git", "commit", "-m", "Initial Commit"])
 
@@ -458,6 +472,7 @@ def test_keep_fragments(self, runner):
         call(["git", "init"])
         call(["git", "config", "user.name", "user"])
         call(["git", "config", "user.email", "[email protected]"])
+        call(["git", "config", "commit.gpgSign", "false"])
         call(["git", "add", "."])
         call(["git", "commit", "-m", "Initial Commit"])
 
@@ -491,6 +506,7 @@ def test_yes_keep_error(self, runner):
         call(["git", "init"])
         call(["git", "config", "user.name", "user"])
         call(["git", "config", "user.email", "[email protected]"])
+        call(["git", "config", "commit.gpgSign", "false"])
         call(["git", "add", "."])
         call(["git", "commit", "-m", "Initial Commit"])
 
@@ -519,6 +535,7 @@ def test_confirmation_says_no(self):
             call(["git", "init"])
             call(["git", "config", "user.name", "user"])
             call(["git", "config", "user.email", "[email protected]"])
+            call(["git", "config", "commit.gpgSign", "false"])
             call(["git", "add", "."])
             call(["git", "commit", "-m", "Initial Commit"])
 

src/towncrier/test/test_builder.py

@@ -8,63 +8,73 @@
 
 class TestParseNewsfragmentBasename(TestCase):
     def test_simple(self):
+        """<number>.<category> 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):
+        """<number>.<category>.<counter> 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),
+        )