Skip to content

Commit

Permalink
Fix create for a bare orphan in a section not adding random hex (#468)
Browse files Browse the repository at this point in the history
* Fix `create` for a bare orphan in a section not adding random hex

* Add change

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Make test_create_orphane_fragment os independent

* Add tests for a custom orphan prefix

* Fix a test to actually test what it should test

* This ain't Markdown

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hynek Schlawack <hs@ox.cx>
  • Loading branch information
3 people authored Feb 9, 2023
1 parent 53dd3e2 commit 566d315
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 15 deletions.
11 changes: 9 additions & 2 deletions src/towncrier/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,16 @@ def __main(
"""
base_directory, config = load_config_from_options(directory, config_path)

if config.orphan_prefix and filename.startswith(f"{config.orphan_prefix}."):
file_dir, file_basename = os.path.split(filename)
if config.orphan_prefix and file_basename.startswith(f"{config.orphan_prefix}."):
# Append a random hex string to the orphan news fragment base name.
filename = f"{config.orphan_prefix}{os.urandom(4).hex()}{filename[1:]}"
filename = os.path.join(
file_dir,
(
f"{config.orphan_prefix}{os.urandom(4).hex()}"
f"{file_basename[len(config.orphan_prefix):]}"
),
)
if len(filename.split(".")) < 2 or (
filename.split(".")[-1] not in config.types
and filename.split(".")[-2] not in config.types
Expand Down
3 changes: 3 additions & 0 deletions src/towncrier/newsfragments/468.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix creating fragment in a section not adding random characters.

For example, ``towncrier create some_section/+.feature`` should end up as a fragment named something like ``news/some_section/+a4e22da1.feature``.
61 changes: 48 additions & 13 deletions src/towncrier/test/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
# See LICENSE for details.

import os
import string

from pathlib import Path
from textwrap import dedent
from unittest import mock

from click.testing import CliRunner
from twisted.trial.unittest import TestCase

from ..create import _main
from .helpers import setup_simple_project
from .helpers import setup_simple_project, with_isolated_runner


class TestCli(TestCase):
Expand Down Expand Up @@ -164,25 +166,58 @@ def test_file_exists(self):
self.assertEqual(type(result.exception), SystemExit)
self.assertIn("123.feature.rst already exists", result.output)

def test_create_orphan_fragment(self):
@with_isolated_runner
def test_create_orphan_fragment(self, runner: CliRunner):
"""
When a fragment starts with the only the orphan prefix (``+`` by default), the
create CLI automatically extends the new file's base name to contain a random
value to avoid commit collisions.
"""
runner = CliRunner()
setup_simple_project()

with runner.isolated_filesystem():
setup_simple_project()
frag_path = Path("foo", "newsfragments")
sub_frag_path = frag_path / "subsection"
sub_frag_path.mkdir()

self.assertEqual([], os.listdir("foo/newsfragments"))
result = runner.invoke(_main, ["+.feature"])
self.assertEqual(0, result.exit_code)
result = runner.invoke(
_main, [str(Path("subsection", "+.feature"))], catch_exceptions=False
)
self.assertEqual(0, result.exit_code, result.output)

fragments = [p for p in frag_path.rglob("*") if p.is_file()]
self.assertEqual(2, len(fragments))
change1, change2 = fragments

runner.invoke(_main, ["+.feature"])
fragments = os.listdir("foo/newsfragments")
self.assertEqual(change1.suffix, ".feature")
self.assertTrue(change1.stem.startswith("+"))
# Length should be '+' character and 8 random hex characters.
self.assertEqual(len(change1.stem), 9)

self.assertEqual(1, len(fragments))
filename = fragments[0]
self.assertTrue(filename.endswith(".feature"))
self.assertTrue(filename.startswith("+"))
self.assertEqual(change2.suffix, ".feature")
self.assertTrue(change2.stem.startswith("+"))
self.assertEqual(change2.parent, sub_frag_path)
# Length should be '+' character and 8 random hex characters.
self.assertEqual(len(filename.split(".")[0]), 9)
self.assertEqual(len(change2.stem), 9)

@with_isolated_runner
def test_create_orphan_fragment_custom_prefix(self, runner: CliRunner):
"""
Check that the orphan prefix can be customized.
"""
setup_simple_project(extra_config='orphan_prefix = "$$$"')

frag_path = Path("foo", "newsfragments")

result = runner.invoke(_main, ["$$$.feature"])
self.assertEqual(0, result.exit_code, result.output)

fragments = list(frag_path.rglob("*"))
self.assertEqual(len(fragments), 1)
change = fragments[0]
self.assertTrue(change.stem.startswith("$$$"))
# Length should be '$$$' characters and 8 random hex characters.
self.assertEqual(len(change.stem), 11)
# Check the remainder are all hex characters.
self.assertTrue(all(c in string.hexdigits for c in change.stem[3:]))

0 comments on commit 566d315

Please sign in to comment.