Merge pull request #629 from neerdoc/template_fixes

Fixed problem with multiple templates for different documents. This will now give an error.
Fixed problem with doc attribute not being defined.
Updated a bunch of the actions in the runners due to node16 going obsolete.
Added an automation on the test coverage so that clear feedback is given to contributors if their code changes are not covered by tests.

.github/workflows/change-coverage.yml

@@ -0,0 +1,48 @@
+name: Execute tests
+on:
+  workflow_call:
+    inputs:
+      basepath:
+        required: false
+        type: string
+      os:
+        required: true
+        type: string
+      workpath:
+        required: true
+        type: string
+
+jobs:
+  change-coverage:
+    runs-on: ubuntu-latest
+    name: Change coverage
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+          architecture: x64
+
+      - uses: Gr1N/setup-poetry@v8
+
+      - name: Check system dependencies
+        run: make doctor
+
+      - uses: actions/cache@v4
+        with:
+          path: .venv
+          key: ${{ runner.os }}-poetry-${{ hashFiles('poetry.lock') }}
+
+      - name: Install project dependencies
+        run: make install
+
+      - name: Check coverage
+        run: |
+          TEST_INTEGRATION=true poetry run pytest doorstop --doctest-modules --cov=doorstop --cov-report=xml --cov-report=term-missing
+          git fetch origin develop:develop
+          # TEST_INTEGRATION=true poetry run diff-cover ./coverage.xml --fail-under=100 --compare-branch=develop
+          TEST_INTEGRATION=true poetry run diff-cover ./coverage.xml --fail-under=100 --compare-branch=$(git for-each-ref --sort=-committerdate refs/heads/develop | cut -f 1 -d ' ')

.github/workflows/execute-tests.yml

@@ -27,7 +27,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
 
       - name: Change path on Windows
         if: ${{ inputs.os == 'windows-latest' }}
@@ -38,7 +38,7 @@ jobs:
           mkdir -p ${{ inputs.workpath }}
           mv $env:GITHUB_WORKSPACE\* ${{ inputs.workpath }}\ -Force
 
-      - uses: actions/setup-python@v2
+      - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
           architecture: x64
@@ -48,7 +48,7 @@ jobs:
       - name: Check system dependencies
         run: make doctor
 
-      - uses: actions/cache@v2
+      - uses: actions/cache@v4
         with:
           path: .venv
           key: ${{ runner.os }}-poetry-${{ hashFiles('poetry.lock') }}
@@ -60,7 +60,7 @@ jobs:
       # very slow. Especially any client/server tests seems to be problematic.
       # This is a simple attempt to re-run the tests up to three times if they
       # fail. Does not add any execution time if successful.
-      - uses: Wandalen/wretry.action@v1.0.20
+      - uses: Wandalen/wretry.action@v1.4.4
         name: Run tests
         with:
           command: make test

.github/workflows/test-linux.yml

@@ -6,6 +6,11 @@ on:
     branches: [ develop ]
 
 jobs:
+  Coverage:
+    uses: ./.github/workflows/change-coverage.yml
+    with:
+      os: "ubuntu-latest"
+      workpath: "/home/runner/work/doorstop/doorstop"
   Test:
     uses: ./.github/workflows/execute-tests.yml
     with:

.gitignore

@@ -11,6 +11,7 @@ Icon*
 # Temporary virtual environment files
 /.cache/
 /.venv/
+/.python-version
 
 # Temporary server files
 .env

Makefile

@@ -116,7 +116,7 @@ test-cover: install
 # Run first to generate coverage data current code.
 	TEST_INTEGRATION=true poetry run pytest doorstop --doctest-modules --cov=doorstop --cov-report=xml --cov-report=term-missing
 # Run second to generate coverage data for the code in the develop branch.
-	TEST_INTEGRATION=true diff-cover ./coverage.xml --compare-branch=$(shell git for-each-ref --sort=-committerdate refs/heads/develop | cut -f 1 -d ' ')
+	TEST_INTEGRATION=true poetry run diff-cover ./coverage.xml --fail-under=100 --compare-branch=$(shell git for-each-ref --sort=-committerdate refs/heads/develop | cut -f 1 -d ' ')
 
 .PHONY: read-coverage
 read-coverage:

doorstop/cli/tests/test_all.py

@@ -21,6 +21,7 @@
 )
 from doorstop.core.builder import _clear_tree
 from doorstop.core.document import Document
+from doorstop.core.tests.helpers import on_error_with_retry
 
 REQ_COUNT = 23
 ALL_COUNT = 55
@@ -36,7 +37,7 @@ def setUp(self):
     def tearDown(self):
         os.chdir(self.cwd)
         if os.path.exists(self.temp):
-            shutil.rmtree(self.temp)
+            shutil.rmtree(self.temp, onerror=on_error_with_retry)
 
 
 class MockTestCase(TempTestCase):
@@ -62,7 +63,7 @@ def setUp(self):
     def tearDown(self):
         super().tearDown()
         os.chdir(self.cwd)
-        shutil.rmtree(self.temp)
+        shutil.rmtree(self.temp, onerror=on_error_with_retry)
 
     def test_main(self):
         """Verify 'doorstop' can be called."""

doorstop/core/publishers/base.py

@@ -320,22 +320,23 @@ def get_document_attributes(obj, is_html=False, extensions=None):
     except AttributeError:
         attribute_defaults = None
     if attribute_defaults:
-        if "name" in attribute_defaults["doc"]:
-            # Name should only be set if it is not empty.
-            if attribute_defaults["doc"]["name"] != "":
-                doc_attributes["name"] = attribute_defaults["doc"]["name"]
-        if "title" in attribute_defaults["doc"]:
-            doc_attributes["title"] = attribute_defaults["doc"]["title"]
-        if "ref" in attribute_defaults["doc"]:
-            doc_attributes["ref"] = attribute_defaults["doc"]["ref"]
-        if "by" in attribute_defaults["doc"]:
-            doc_attributes["by"] = attribute_defaults["doc"]["by"]
-        if "major" in attribute_defaults["doc"]:
-            doc_attributes["major"] = attribute_defaults["doc"]["major"]
-        if "minor" in attribute_defaults["doc"]:
-            doc_attributes["minor"] = attribute_defaults["doc"]["minor"]
-        if "copyright" in attribute_defaults["doc"]:
-            doc_attributes["copyright"] = attribute_defaults["doc"]["copyright"]
+        if "doc" in attribute_defaults:
+            if "name" in attribute_defaults["doc"]:
+                # Name should only be set if it is not empty.
+                if attribute_defaults["doc"]["name"] != "":
+                    doc_attributes["name"] = attribute_defaults["doc"]["name"]
+            if "title" in attribute_defaults["doc"]:
+                doc_attributes["title"] = attribute_defaults["doc"]["title"]
+            if "ref" in attribute_defaults["doc"]:
+                doc_attributes["ref"] = attribute_defaults["doc"]["ref"]
+            if "by" in attribute_defaults["doc"]:
+                doc_attributes["by"] = attribute_defaults["doc"]["by"]
+            if "major" in attribute_defaults["doc"]:
+                doc_attributes["major"] = attribute_defaults["doc"]["major"]
+            if "minor" in attribute_defaults["doc"]:
+                doc_attributes["minor"] = attribute_defaults["doc"]["minor"]
+            if "copyright" in attribute_defaults["doc"]:
+                doc_attributes["copyright"] = attribute_defaults["doc"]["copyright"]
     # Check output format. If html we need go convert from markdown.
     if is_html:
         # Only convert title and copyright.

doorstop/core/publishers/tests/test_publisher_html.py

@@ -5,6 +5,7 @@
 # pylint: disable=unused-argument,protected-access
 
 import os
+import stat
 import unittest
 from secrets import token_hex
 from shutil import rmtree
@@ -22,6 +23,7 @@
     MockDocument,
     MockItem,
 )
+from doorstop.core.tests.helpers import on_error_with_retry
 from doorstop.core.types import UID
 
 
@@ -37,7 +39,7 @@ def setUp(self):
     @classmethod
     def tearDownClass(cls):
         """Remove test folder."""
-        rmtree("mock_%s" % __name__)
+        rmtree("mock_%s" % __name__, onerror=on_error_with_retry)
 
     @patch("os.path.isdir", Mock(side_effect=[False, False, False, False]))
     @patch("os.makedirs")

doorstop/core/publishers/tests/test_publisher_html_doc.py

@@ -5,6 +5,7 @@
 # pylint: disable=unused-argument,protected-access
 
 import os
+import stat
 import unittest
 from secrets import token_hex
 from shutil import rmtree
@@ -14,6 +15,7 @@
 from doorstop.core.builder import build
 from doorstop.core.publishers.tests.helpers import HTML_TEMPLATE_WALK, getWalk
 from doorstop.core.tests import ROOT, MockDataMixIn
+from doorstop.core.tests.helpers import on_error_with_retry
 
 
 class TestPublisherFullDocument(MockDataMixIn, unittest.TestCase):
@@ -44,7 +46,7 @@ def setUp(self):
     @classmethod
     def tearDownClass(cls):
         """Remove test folder."""
-        rmtree("mock_%s" % __name__)
+        rmtree("mock_%s" % __name__, onerror=on_error_with_retry)
 
     def test_publish_html_tree_copies_assets(self):
         """Verify that html assets are published when publishing a tree."""

doorstop/core/publishers/tests/test_publisher_latex_doc.py

@@ -5,6 +5,7 @@
 # pylint: disable=unused-argument,protected-access
 
 import os
+import stat
 import unittest
 from secrets import token_hex
 from shutil import rmtree
@@ -21,6 +22,7 @@
     YAML_LATEX_ONLY_REF,
 )
 from doorstop.core.tests import ROOT, MockDataMixIn, MockDocument
+from doorstop.core.tests.helpers import on_error_with_retry
 
 
 class TestPublisherFullDocument(MockDataMixIn, unittest.TestCase):
@@ -58,7 +60,7 @@ def setUp(self):
     @classmethod
     def tearDownClass(cls):
         """Remove test folder."""
-        rmtree("mock_%s" % __name__)
+        rmtree("mock_%s" % __name__, onerror=on_error_with_retry)
 
     def test_publish_latex_tree_copies_assets(self):
         """Verify that LaTeX assets are published when publishing a tree."""

doorstop/core/publishers/tests/test_publisher_markdown.py

@@ -5,6 +5,7 @@
 # pylint: disable=unused-argument,protected-access
 
 import os
+import stat
 import unittest
 from secrets import token_hex
 from shutil import rmtree
@@ -20,6 +21,7 @@
     MockItem,
     MockItemAndVCS,
 )
+from doorstop.core.tests.helpers import on_error_with_retry
 
 
 class TestModule(MockDataMixIn, unittest.TestCase):
@@ -35,7 +37,7 @@ def setUp(self):
     def tearDownClass(cls):
         """Remove test folder."""
         if os.path.exists("mock_%s" % __name__):
-            rmtree("mock_%s" % __name__)
+            rmtree("mock_%s" % __name__, onerror=on_error_with_retry)
 
     def test_lines_markdown_item(self):
         """Verify Markdown can be published from an item."""

doorstop/core/publishers/tests/test_publisher_markdown_doc.py

@@ -5,6 +5,7 @@
 # pylint: disable=unused-argument,protected-access
 
 import os
+import stat
 import unittest
 from secrets import token_hex
 from shutil import rmtree
@@ -13,6 +14,7 @@
 from doorstop.core.builder import build
 from doorstop.core.publishers.tests.helpers import getWalk
 from doorstop.core.tests import ROOT, MockDataMixIn, MockDocument
+from doorstop.core.tests.helpers import on_error_with_retry
 
 
 class TestPublisherFullDocument(MockDataMixIn, unittest.TestCase):
@@ -40,7 +42,7 @@ def setUp(self):
     @classmethod
     def tearDownClass(cls):
         """Remove test folder."""
-        rmtree("mock_%s" % __name__)
+        rmtree("mock_%s" % __name__, onerror=on_error_with_retry)
 
     def test_publish_markdown_tree_copies_assets(self):
         """Verify that markdown assets are published when publishing a tree."""

doorstop/core/publishers/tests/test_publisher_text.py

@@ -5,6 +5,7 @@
 # pylint: disable=unused-argument,protected-access
 
 import os
+import stat
 import unittest
 from secrets import token_hex
 from shutil import rmtree
@@ -13,6 +14,7 @@
 from doorstop.core import publisher
 from doorstop.core.publishers.tests.helpers import getLines
 from doorstop.core.tests import MockDataMixIn, MockItemAndVCS
+from doorstop.core.tests.helpers import on_error_with_retry
 
 
 class TestModule(MockDataMixIn, unittest.TestCase):
@@ -28,7 +30,7 @@ def setUp(self):
     def tearDownClass(cls):
         """Remove test folder."""
         if os.path.exists("mock_%s" % __name__):
-            rmtree("mock_%s" % __name__)
+            rmtree("mock_%s" % __name__, onerror=on_error_with_retry)
 
     def test_lines_text_item(self):
         """Verify text can be published from an item."""

doorstop/core/template.py

@@ -53,7 +53,17 @@ def get_template(obj, path, ext, template):
     output_dir = path
 
     if is_tree(obj):
-        document_template = obj.documents[0].template
+        document_template = None
+        template_count = 0
+        for each in obj.documents:
+            if each.template:
+                document_template = each.template
+                template_count += 1
+        if template_count > 1:
+            raise common.DoorstopError(
+                """Multiple templates found in tree. Please specify a single template for the tree.
+I.e., only one of the documents in the tree should have a template folder."""
+            )
     else:
         document_template = obj.template
 

doorstop/core/tests/helpers.py

@@ -3,7 +3,10 @@
 
 """Unit test helper functions to reduce code duplication."""
 from logging import NullHandler
+from os import chmod
 from shutil import copytree
+from stat import S_IWRITE
+from time import sleep
 from typing import List
 
 from doorstop.core.builder import build
@@ -33,3 +36,29 @@ def build_expensive_tree(obj):
     # Build a tree.
     copytree(ROOT, obj.datapath)
     obj.mock_tree = build(cwd=obj.datapath, root=obj.datapath, request_next_number=None)
+
+
+def on_error_with_retry(func, path, exc_info, retries=60):
+    """Define a separate function to handle errors for rmtree.
+
+    This callback function is used to retry rmtree operations
+    that fail for up to 60 seconds. This is necessary because
+    Windows does not always release file handles immediately,
+    and the rmtree operation can fail.
+    """
+    if retries > 0:
+        # Change the file permissions
+        chmod(path, S_IWRITE)
+        # Sleep for 1 seconds to wait for the race condition to resolve
+        sleep(1)
+        try:
+            # Try the operation again
+            func(path)
+        # pylint: disable=broad-except
+        except Exception as e:
+            # If an error occurs, recurse with one fewer retry available
+            print(f"Retry {5 - retries} failed for {path}, error: {e}. Retrying...")
+            on_error_with_retry(func, path, exc_info, retries - 1)
+    else:
+        # If no retries are left, raise an exception or handle the final failure case
+        print(f"Failed to remove {path} after multiple retries.")