Skip to content

Commit

Permalink
Check for invalid fragment names
Browse files Browse the repository at this point in the history
  • Loading branch information
MetRonnie committed Jul 12, 2024
1 parent f60e750 commit b85391c
Show file tree
Hide file tree
Showing 9 changed files with 134 additions and 2 deletions.
7 changes: 7 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,13 @@ Top level keys

``true`` by default.

``build_ignore_filenames``
A list of filenames in the news fragments directory to ignore when building the news file.

If this is configured, it turns on the ``--strict`` build mode which will fail if there are any news fragment files not in this list that have invalid filenames. Note that if a template is used, it should also be included here.

``None`` by default.

Extra top level keys for Python projects
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
5 changes: 5 additions & 0 deletions docs/tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ Create this folder::

The ``.gitignore`` will remain and keep Git from not tracking the directory.

If needed, you can also specify a list of filenames for ``towncrier`` to ignore in the news fragments directory::

[tool.towncrier]
build_ignore_filenames = ["README.rst"]


Detecting Version
-----------------
Expand Down
23 changes: 23 additions & 0 deletions src/towncrier/_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pathlib import Path
from typing import Any, DefaultDict, Iterable, Iterator, Mapping, NamedTuple, Sequence

from click import ClickException
from jinja2 import Template

from towncrier._settings.load import Config
Expand Down Expand Up @@ -106,10 +107,22 @@ def __call__(self, section_directory: str = "") -> str:
def find_fragments(
base_directory: str,
config: Config,
strict: bool | None,
) -> tuple[Mapping[str, Mapping[tuple[str, str, int], str]], list[tuple[str, str]]]:
"""
Sections are a dictonary of section names to paths.
In strict mode, raise ClickException if any fragments have an invalid name.
"""
if strict is None:
# If strict mode is not set, turn it on only if build_ignore_filenames is set
# (this maintains backward compatibility).
strict = config.build_ignore_filenames is not None

ignored_files = {".gitignore"}
if config.build_ignore_filenames:
ignored_files.update(config.build_ignore_filenames)

get_section_path = FragmentsPath(base_directory, config)

content = {}
Expand All @@ -129,10 +142,20 @@ def find_fragments(
file_content = {}

for basename in files:
# Skip files that are deliberately ignored
if basename in ignored_files:
continue

issue, category, counter = parse_newfragment_basename(
basename, config.types
)
if category is None:
if strict and issue is None:
raise ClickException(
f"Invalid news fragment name: {basename}\n"
"If this filename is deliberate, add it to "
"'build_ignore_filenames' in your configuration."
)
continue
assert issue is not None
assert counter is not None
Expand Down
1 change: 1 addition & 0 deletions src/towncrier/_settings/load.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Config:
orphan_prefix: str = "+"
create_eof_newline: bool = True
create_add_extension: bool = True
build_ignore_filenames: list[str] | None = None


class ConfigError(ClickException):
Expand Down
16 changes: 15 additions & 1 deletion src/towncrier/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ def _validate_answer(ctx: Context, param: Option, value: bool) -> bool:
help="Do not ask for confirmations. But keep news fragments.",
callback=_validate_answer,
)
@click.option(
"--strict",
"strict",
default=None,
flag_value=True,
help=(
"Fail if there are any news fragments that have invalid filenames (this is "
"automatically turned on if 'build_ignore_filenames' has been set in the "
"configuration)."
),
)
def _main(
draft: bool,
directory: str | None,
Expand All @@ -115,6 +126,7 @@ def _main(
project_date: str | None,
answer_yes: bool,
answer_keep: bool,
strict: bool | None,
) -> None:
"""
Build a combined news file from news fragment.
Expand All @@ -129,6 +141,7 @@ def _main(
project_date,
answer_yes,
answer_keep,
strict,
)
except ConfigError as e:
print(e, file=sys.stderr)
Expand All @@ -144,6 +157,7 @@ def __main(
project_date: str | None,
answer_yes: bool,
answer_keep: bool,
strict: bool | None,
) -> None:
"""
The main entry point.
Expand Down Expand Up @@ -178,7 +192,7 @@ def __main(

click.echo("Finding news fragments...", err=to_err)

fragment_contents, fragment_files = find_fragments(base_directory, config)
fragment_contents, fragment_files = find_fragments(base_directory, config, strict)
fragment_filenames = [filename for (filename, _category) in fragment_files]

click.echo("Rendering news fragments...", err=to_err)
Expand Down
4 changes: 3 additions & 1 deletion src/towncrier/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,14 @@ def __main(
click.echo(f"{n}. {change}")
click.echo("----")

# This will fail if any fragment files have an invalid name:
all_fragment_files = find_fragments(base_directory, config, strict=True)[1]

news_file = os.path.normpath(os.path.join(base_directory, config.filename))
if news_file in files:
click.echo("Checks SKIPPED: news file changes detected.")
sys.exit(0)

all_fragment_files = find_fragments(base_directory, config)[1]
fragments = set() # will only include fragments of types that are checked
unchecked_fragments = set() # will include fragments of types that are not checked
for fragment_filename, category in all_fragment_files:
Expand Down
3 changes: 3 additions & 0 deletions src/towncrier/newsfragments/622.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
``towncrier check`` will now fail if any news fragments have invalid filenames.

Added a new configuration option called ``build_ignore_filenames`` that allows you to specify a list of filenames that should be ignored. If this is set, ``towncrier build`` will also fail if any filenames are invalid, except for those in the list.
57 changes: 57 additions & 0 deletions src/towncrier/test/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1530,3 +1530,60 @@ def test_orphans_in_non_showcontent_markdown(self, runner):

self.assertEqual(0, result.exit_code, result.output)
self.assertEqual(expected_output, result.output)

@with_project(
config="""
[tool.towncrier]
package = "foo"
build_ignore_filenames = ["template.jinja", "CAPYBARAS.md"]
"""
)
def test_invalid_fragment_names(self, runner):
"""
When build_ignore_filenames is set, files with those names are ignored.
"""
opts = ["--draft", "--date", "01-01-2001", "--version", "1.0.0"]
# Valid filename:
with open("foo/newsfragments/123.feature", "w") as f:
f.write("Adds levitation")
# Files that should be ignored:
with open("foo/newsfragments/template.jinja", "w") as f:
f.write("Jinja template")
with open("foo/newsfragments/CAPYBARAS.md", "w") as f:
f.write("Peanut butter")
# Automatically ignored:
with open("foo/newsfragments/.gitignore", "w") as f:
f.write("!.gitignore")

result = runner.invoke(_main, opts)
# Should succeed
self.assertEqual(0, result.exit_code, result.output)

# Invalid filename:
with open("foo/newsfragments/feature.124", "w") as f:
f.write("Extends levitation")

result = runner.invoke(_main, opts)
# Should now fail
self.assertEqual(1, result.exit_code, result.output)
self.assertIn("Invalid news fragment name: feature.124", result.output)

@with_project()
def test_invalid_fragment_names_strict(self, runner):
"""
When using --strict, any invalid filenames will cause an error even if
build_ignore_filenames is NOT set.
"""
opts = ["--draft", "--date", "01-01-2001", "--version", "1.0.0"]
# Invalid filename:
with open("foo/newsfragments/feature.124", "w") as f:
f.write("Extends levitation")

result = runner.invoke(_main, opts)
# Should succeed in normal mode
self.assertEqual(0, result.exit_code, result.output)

result = runner.invoke(_main, [*opts, "--strict"])
# Should now fail
self.assertEqual(1, result.exit_code, result.output)
self.assertIn("Invalid news fragment name: feature.124", result.output)
20 changes: 20 additions & 0 deletions src/towncrier/test/test_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,3 +470,23 @@ def test_in_different_dir_with_nondefault_newsfragments_directory(self, runner):
self.assertTrue(
result.output.endswith("No new newsfragments found on this branch.\n")
)

@with_isolated_runner
def test_invalid_fragment_name(self, runner):
create_project("pyproject.toml")
opts = ["--compare-with", "main"]

write("foo/bar.py", "# Scorpions!")
write("foo/newsfragments/123.feature", "Adds scorpions")
write("foo/newsfragments/.gitignore", "!.gitignore")
commit("add stuff")

result = runner.invoke(towncrier_check, opts)
self.assertEqual(0, result.exit_code, result.output)

# Make invalid filename:
os.rename("foo/newsfragments/123.feature", "foo/newsfragments/feature.123")

result = runner.invoke(towncrier_check, opts)
self.assertEqual(1, result.exit_code, result.output)
self.assertIn("Invalid news fragment name: feature.123", result.output)

0 comments on commit b85391c

Please sign in to comment.