Merge pull request #337 from hexagonrecursion/check-release

Skip the checks if the news file was changed

README.rst

@@ -163,3 +163,17 @@ Furthermore, you can add your own fragment types using:
     directory = "deprecation"
     name = "Deprecations"
     showcontent = true
+
+
+Automatic pull request checks
+-----------------------------
+
+To check if a feature branch adds at least one news fragment, run::
+
+    towncrier check
+
+By default this compares the current branch against ``origin/master``. You can use ``--compare-with`` if the trunk is named differently::
+
+    towncrier check --compare-with origin/main
+
+The check is automatically skipped when the main news file is modified inside the branch as this signals a release branch that is expected to not have news fragments.

src/towncrier/check.py

@@ -52,7 +52,7 @@ def __main(comparewith, directory, config):
         raise
 
     if not files_changed:
-        click.echo("On trunk, or no diffs, so no newsfragment required.")
+        click.echo("On {} branch, or no diffs, so no newsfragment required.".format(comparewith))
         sys.exit(0)
 
     files = {
@@ -66,6 +66,11 @@ def __main(comparewith, directory, config):
         click.echo("{}. {}".format(n, change))
     click.echo("----")
 
+    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)
+
     if config.get("directory"):
         fragment_base_directory = os.path.abspath(config["directory"])
         fragment_directory = None

src/towncrier/newsfragments/337.feature.rst

@@ -0,0 +1,5 @@
+Make the check subcommand succeed for branches that change the news file
+
+This should enable the ``check`` subcommand to be used as a CI lint step and
+not fail when a pull request only modifies the configured news file (i.e. when
+the news file is being assembled for the next release).

src/towncrier/test/test_check.py

@@ -2,18 +2,26 @@
 # See LICENSE for details.
 
 import os
+import os.path
 import sys
 
 from twisted.trial.unittest import TestCase
 from click.testing import CliRunner
 from subprocess import call, Popen, PIPE
 
-from towncrier.check import _main
+from towncrier.check import _main as towncrier_check
 
 
-def create_project(pyproject_path):
+def create_project(pyproject_path="pyproject.toml", main_branch="main"):
+    """
+    Create the project files in the main branch that already has a
+    news-fragment and then switch to a new in-work branch.
+    """
     with open(pyproject_path, "w") as f:
-        f.write("[tool.towncrier]\n" 'package = "foo"\n')
+        f.write(
+            "[tool.towncrier]\n"
+            'package = "foo"\n'
+            )
     os.mkdir("foo")
     with open("foo/__init__.py", "w") as f:
         f.write('__version__ = "1.2.3"\n')
@@ -22,12 +30,47 @@ def create_project(pyproject_path):
     with open(fragment_path, "w") as f:
         f.write("Adds levitation")
 
-    call(["git", "init"])
+    initial_commit(branch=main_branch)
+    call(["git", "checkout", "-b", "otherbranch"])
+
+
+def commit(message):
+    """Stage and commit the repo in the current working directory
+
+    There must be uncommitted changes otherwise git will complain:
+    "nothing to commit, working tree clean"
+    """
+    call(["git", "add", "."])
+    call(["git", "commit", "-m", message])
+
+
+def write(path, contents):
+    """Create a file with given contents including any missing parent directories"""
+    dir = os.path.dirname(path)
+    if dir:
+        try:
+            os.makedirs(dir)
+        except OSError:  # pragma: no cover
+            pass
+    with open(path, "w") as f:
+        f.write(contents)
+
+
+def initial_commit(branch='main'):
+    """
+    Create a git repo, configure it and make an initial commit
+
+    There must be uncommitted changes otherwise git will complain:
+    "nothing to commit, working tree clean"
+    """
+    # --initial-branch is explicitly set to `main` because
+    # git has deprecated the default branch name.
+    call(["git", "init", "--initial-branch={}".format(branch)])
+    # Without ``git config` user.name and user.email `git commit` fails
+    # unless the settings are set globally
     call(["git", "config", "user.name", "user"])
     call(["git", "config", "user.email", "user@example.com"])
-    call(["git", "add", "."])
-    call(["git", "commit", "-m", "Initial Commit"])
-    call(["git", "checkout", "-b", "otherbranch"])
+    commit("Initial Commit")
 
 
 class TestChecker(TestCase):
@@ -42,7 +85,7 @@ def test_git_fails(self):
         with runner.isolated_filesystem():
             create_project("pyproject.toml")
 
-            result = runner.invoke(_main, ["--compare-with", "hblaugh"])
+            result = runner.invoke(towncrier_check, ["--compare-with", "hblaugh"])
             self.assertIn("git produced output while failing", result.output)
             self.assertIn("hblaugh", result.output)
 
@@ -61,23 +104,26 @@ def test_no_changes_made_config_path(self):
         )
 
     def _test_no_changes_made(self, pyproject_path, invoke):
+        """
+        When no changes are made on a new branch, no checks are performed.
+        """
         runner = CliRunner()
 
         with runner.isolated_filesystem():
-            create_project(pyproject_path)
+            create_project(pyproject_path, main_branch="master")
 
-            result = invoke(runner, _main, ["--compare-with", "master"])
+            result = invoke(runner, towncrier_check, ["--compare-with", "master"])
 
             self.assertEqual(0, result.exit_code, result.output)
             self.assertEqual(
-                "On trunk, or no diffs, so no newsfragment required.\n", result.output
+                "On master branch, or no diffs, so no newsfragment required.\n", result.output
             )
 
     def test_fragment_exists(self):
         runner = CliRunner()
 
         with runner.isolated_filesystem():
-            create_project("pyproject.toml")
+            create_project("pyproject.toml", main_branch="master")
 
             file_path = "foo/somefile.py"
             with open(file_path, "w") as f:
@@ -93,7 +139,7 @@ def test_fragment_exists(self):
             call(["git", "add", fragment_path])
             call(["git", "commit", "-m", "add a newsfragment"])
 
-            result = runner.invoke(_main, ["--compare-with", "master"])
+            result = runner.invoke(towncrier_check, ["--compare-with", "master"])
 
             self.assertTrue(
                 result.output.endswith(
@@ -107,7 +153,7 @@ def test_fragment_missing(self):
         runner = CliRunner()
 
         with runner.isolated_filesystem():
-            create_project("pyproject.toml")
+            create_project("pyproject.toml", main_branch="master")
 
             file_path = "foo/somefile.py"
             with open(file_path, "w") as f:
@@ -116,7 +162,7 @@ def test_fragment_missing(self):
             call(["git", "add", "foo/somefile.py"])
             call(["git", "commit", "-m", "add a file"])
 
-            result = runner.invoke(_main, ["--compare-with", "master"])
+            result = runner.invoke(towncrier_check, ["--compare-with", "master"])
 
             self.assertEqual(1, result.exit_code)
             self.assertTrue(
@@ -130,7 +176,7 @@ def test_none_stdout_encoding_works(self):
         runner = CliRunner()
 
         with runner.isolated_filesystem():
-            create_project("pyproject.toml")
+            create_project("pyproject.toml", main_branch="master")
 
             fragment_path = "foo/newsfragments/1234.feature"
             with open(fragment_path, "w") as f:
@@ -148,3 +194,73 @@ def test_none_stdout_encoding_works(self):
 
         self.assertEqual(0, proc.returncode)
         self.assertEqual(b"", stderr)
+
+    def test_first_release(self):
+        """
+        The checks should be skipped on a branch that creates the news file.
+
+        If the checks are not skipped in this case, towncrier check would fail
+        for the first release that has a changelog.
+        """
+        runner = CliRunner()
+
+        with runner.isolated_filesystem():
+            # Arrange
+            create_project()
+            # Before any release, the NEWS file might no exist.
+            self.assertNotIn('NEWS.rst', os.listdir('.'))
+
+            call(["towncrier", "build", "--yes", "--version", "1.0"])
+            commit("Prepare a release")
+            # When missing,
+            # the news file is automatically created with a new release.
+            self.assertIn('NEWS.rst', os.listdir('.'))
+
+            # Act
+            result = runner.invoke(towncrier_check, ["--compare-with", "main"])
+
+            # Assert
+            self.assertEqual(0, result.exit_code, (result, result.output))
+            self.assertIn("Checks SKIPPED: news file changes detected", result.output)
+
+    def test_release_branch(self):
+        """
+        The checks for missing news fragments are skipped on a branch that
+        modifies the news file.
+        This is a hint that we are on a release branch
+        and at release time is expected no not have news-fragment files.
+        """
+        runner = CliRunner()
+
+        with runner.isolated_filesystem():
+            # Arrange
+            create_project()
+
+            # Do a first release without any checks.
+            # And merge the release branch back into the main branch.
+            call(["towncrier", "build",  "--yes", "--version", "1.0"])
+            commit("First release")
+            # The news file is now created.
+            self.assertIn('NEWS.rst', os.listdir('.'))
+            call(["git", "checkout", "main"])
+            call(["git", "merge", "otherbranch", "-m", "Sync release in main branch."])
+
+            # We have a new feature branch that has a news fragment that
+            # will be merged to the main branch.
+            call(["git", "checkout", "-b", "new-feature-branch"])
+            write("foo/newsfragments/456.feature", "Foo the bar")
+            commit("A feature in the second release.")
+            call(["git", "checkout", "main"])
+            call(["git", "merge", "new-feature-branch", "-m", "Merge new-feature-branch."])
+
+            # We now have the new release branch.
+            call(["git", "checkout", "-b", "next-release"])
+            call(["towncrier", "build", "--yes", "--version", "2.0"])
+            commit("Second release")
+
+            # Act
+            result = runner.invoke(towncrier_check, ["--compare-with", "main"])
+
+            # Assert
+            self.assertEqual(0, result.exit_code, (result, result.output))
+            self.assertIn("Checks SKIPPED: news file changes detected", result.output)