Fix UnboundLocalError when building changes

Split testing of changes builder to its own test file and root.
Improve coverage a little, and add case that fails if a module
directive is included in the rst source.

Correctly handle changesets with a module declared.

Fixes https://github.com/sphinx-doc/sphinx/issues/6134

8 files changed, 134 insertions, 4 deletions

diff --git a/sphinx/builders/changes.py b/sphinx/builders/changes.py
index 308ae9656..aa5fea5a5 100644
--- a/sphinx/builders/changes.py
+++ b/sphinx/builders/changes.py
@@ -85,8 +85,7 @@ class ChangesBuilder(Builder):
                     entry = '<b>%s</b>: <i>%s</i>.' % (descname, ttext)
                 apichanges.append((entry, changeset.docname, changeset.lineno))
             elif descname or changeset.module:
-                if not changeset.module:
-                    module = _('Builtins')
+                module = changeset.module or _('Builtins')
                 if not descname:
                     descname = _('Module level')
                 if context:
diff --git a/tests/roots/test-changes/base.rst b/tests/roots/test-changes/base.rst
new file mode 100644
index 000000000..a1b28398a
--- /dev/null
+++ b/tests/roots/test-changes/base.rst
@@ -0,0 +1,20 @@
+Version markup
+--------------
+
+.. versionadded:: 0.6
+   Some funny **stuff**.
+
+.. versionchanged:: 0.6
+   Even more funny stuff.
+
+.. deprecated:: 0.6
+   Boring stuff.
+
+.. versionadded:: 1.2
+
+   First paragraph of versionadded.
+
+.. versionchanged:: 1.2
+   First paragraph of versionchanged.
+
+   Second paragraph of versionchanged.
diff --git a/tests/roots/test-changes/c-api.rst b/tests/roots/test-changes/c-api.rst
new file mode 100644
index 000000000..22c0c30c6
--- /dev/null
+++ b/tests/roots/test-changes/c-api.rst
@@ -0,0 +1,24 @@
+.. highlightlang:: c
+
+
+Memory
+======
+
+.. c:function:: void* Test_Malloc(size_t n)
+
+   Allocate *n* bytes of memory.
+
+   .. versionchanged:: 0.6
+
+      Can now be replaced with a different allocator.
+
+System
+------
+
+Access to the system allocator.
+
+.. versionadded:: 0.6
+
+.. c:function:: void* Test_SysMalloc(size_t n)
+
+   Allocate *n* bytes of memory using system allocator.
diff --git a/tests/roots/test-changes/conf.py b/tests/roots/test-changes/conf.py
new file mode 100644
index 000000000..94cb629dc
--- /dev/null
+++ b/tests/roots/test-changes/conf.py
@@ -0,0 +1,6 @@
+# -*- coding: utf-8 -*-
+
+project = 'Sphinx ChangesBuilder tests'
+copyright = '2007-2019 by the Sphinx team, see AUTHORS'
+version = '0.6'
+release = '0.6alpha1'
diff --git a/tests/roots/test-changes/contents.rst b/tests/roots/test-changes/contents.rst
new file mode 100644
index 000000000..ced802608
--- /dev/null
+++ b/tests/roots/test-changes/contents.rst
@@ -0,0 +1,13 @@
+Index for ChangesBuilder tests
+==============================
+
+Contents:
+
+.. toctree::
+   :maxdepth: 2
+   :caption: Table of Contents
+   :name: mastertoc
+
+   base
+   c-api
+   library/utils
diff --git a/tests/roots/test-changes/library/utils.rst b/tests/roots/test-changes/library/utils.rst
new file mode 100644
index 000000000..86446995b
--- /dev/null
+++ b/tests/roots/test-changes/library/utils.rst
@@ -0,0 +1,25 @@
+:mod:`utils` --- Fake utilities module for tests
+================================================
+
+.. module:: utils
+   :synopsis: Utility functions
+
+--------------
+
+The :mod:`utils` module is a pretend python module for changes testing.
+
+
+Classes
+-------
+
+.. class:: Path
+
+   Class for handling paths.
+
+   .. versionadded:: 0.5
+
+      Innovative new way to handle paths.
+
+    .. deprecated:: 0.6
+
+       So, that was a bad idea it turns out.
diff --git a/tests/test_build.py b/tests/test_build.py
index 857e3c8b7..e2243a869 100644
--- a/tests/test_build.py
+++ b/tests/test_build.py
@@ -61,13 +61,13 @@ def nonascii_srcdir(request, rootdir, sphinx_test_tempdir):
 
 
 # note: this test skips building docs for some builders because they have independent testcase.
-#       (html, epub, latex, texinfo and manpage)
+#       (html, changes, epub, latex, texinfo and manpage)
 @pytest.mark.parametrize(
     "buildername",
     [
         # note: no 'html' - if it's ok with dirhtml it's ok with html
         'dirhtml', 'singlehtml', 'pickle', 'json', 'text', 'htmlhelp', 'qthelp',
-        'applehelp', 'changes', 'xml', 'pseudoxml', 'linkcheck',
+        'applehelp', 'xml', 'pseudoxml', 'linkcheck',
     ],
 )
 @mock.patch('sphinx.builders.linkcheck.requests.head',
diff --git a/tests/test_build_changes.py b/tests/test_build_changes.py
new file mode 100644
index 000000000..911dcd0d1
--- /dev/null
+++ b/tests/test_build_changes.py
@@ -0,0 +1,43 @@
+# -*- coding: utf-8 -*-
+"""
+    test_build_changes
+    ~~~~~~~~~~~~~~~~~~
+
+    Test the ChangesBuilder class.
+
+    :copyright: Copyright 2007-2019 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+
+import pytest
+
+
+@pytest.mark.sphinx('changes', testroot='changes')
+def test_build(app):
+    app.build()
+
+    # TODO: Use better checking of html content
+    htmltext = (app.outdir / 'changes.html').text()
+    assert 'New in version 0.6: Some funny stuff.' in htmltext
+    assert 'Changed in version 0.6: Even more funny stuff.' in htmltext
+    assert 'Deprecated since version 0.6: Boring stuff.' in htmltext
+
+    path_html = (
+        '<b>Path</b>: <i>deprecated:</i> Deprecated since version 0.6:'
+        ' So, that was a bad idea it turns out.')
+    assert path_html in htmltext
+
+    malloc_html = (
+        '<b>Test_Malloc</b>: <i>changed:</i> Changed in version 0.6:'
+        ' Can now be replaced with a different allocator.</a>')
+    assert malloc_html in htmltext
+
+
+@pytest.mark.sphinx(
+    'changes', testroot='changes', srcdir='changes-none',
+    confoverrides={'version': '0.7', 'release': '0.7b1'})
+def test_no_changes(app, status):
+    app.build()
+
+    assert 'no changes in version 0.7.' in status.getvalue()
+    assert not (app.outdir / 'changes.html').exists()