diff --git a/.github/workflows/change-coverage.yml b/.github/workflows/change-coverage.yml new file mode 100644 index 000000000..7e32465ed --- /dev/null +++ b/.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 ' ') diff --git a/.github/workflows/execute-tests.yml b/.github/workflows/execute-tests.yml index 9f80c5ca6..28836e678 100644 --- a/.github/workflows/execute-tests.yml +++ b/.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 diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index 4bcfc716b..6514a62d6 100644 --- a/.github/workflows/test-linux.yml +++ b/.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: diff --git a/.gitignore b/.gitignore index 2650cc9a7..15824409f 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ Icon* # Temporary virtual environment files /.cache/ /.venv/ +/.python-version # Temporary server files .env diff --git a/Makefile b/Makefile index 33d818a7e..e09db7e4e 100644 --- a/Makefile +++ b/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: diff --git a/doorstop/cli/tests/test_all.py b/doorstop/cli/tests/test_all.py index 05c87983a..60afd97d9 100644 --- a/doorstop/cli/tests/test_all.py +++ b/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.""" diff --git a/doorstop/core/publishers/base.py b/doorstop/core/publishers/base.py index 056ae4e3f..e34d0f0bb 100644 --- a/doorstop/core/publishers/base.py +++ b/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. diff --git a/doorstop/core/publishers/tests/test_publisher_html.py b/doorstop/core/publishers/tests/test_publisher_html.py index 67f9d55f9..9f83c5923 100644 --- a/doorstop/core/publishers/tests/test_publisher_html.py +++ b/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") diff --git a/doorstop/core/publishers/tests/test_publisher_html_doc.py b/doorstop/core/publishers/tests/test_publisher_html_doc.py index 8104defa4..3a947debc 100644 --- a/doorstop/core/publishers/tests/test_publisher_html_doc.py +++ b/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.""" diff --git a/doorstop/core/publishers/tests/test_publisher_latex_doc.py b/doorstop/core/publishers/tests/test_publisher_latex_doc.py index dc0c551dd..da84a1f96 100644 --- a/doorstop/core/publishers/tests/test_publisher_latex_doc.py +++ b/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.""" diff --git a/doorstop/core/publishers/tests/test_publisher_markdown.py b/doorstop/core/publishers/tests/test_publisher_markdown.py index aae1d30a2..88e637ff5 100644 --- a/doorstop/core/publishers/tests/test_publisher_markdown.py +++ b/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.""" diff --git a/doorstop/core/publishers/tests/test_publisher_markdown_doc.py b/doorstop/core/publishers/tests/test_publisher_markdown_doc.py index 247f89eea..0c1af1b41 100644 --- a/doorstop/core/publishers/tests/test_publisher_markdown_doc.py +++ b/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.""" diff --git a/doorstop/core/publishers/tests/test_publisher_text.py b/doorstop/core/publishers/tests/test_publisher_text.py index 9b9c632e1..0223ae8a8 100644 --- a/doorstop/core/publishers/tests/test_publisher_text.py +++ b/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.""" diff --git a/doorstop/core/template.py b/doorstop/core/template.py index 709946899..0da6681cf 100644 --- a/doorstop/core/template.py +++ b/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 diff --git a/doorstop/core/tests/helpers.py b/doorstop/core/tests/helpers.py index 1a80c4757..8ac1c4426 100644 --- a/doorstop/core/tests/helpers.py +++ b/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.") diff --git a/doorstop/core/tests/test_all.py b/doorstop/core/tests/test_all.py index 5030ad2dc..155c325aa 100644 --- a/doorstop/core/tests/test_all.py +++ b/doorstop/core/tests/test_all.py @@ -29,6 +29,7 @@ SYS, DocumentNoSkip, ) +from doorstop.core.tests.helpers import on_error_with_retry from doorstop.core.vcs import mockvcs @@ -387,7 +388,7 @@ def setUp(self): def tearDown(self): os.chdir(self.cwd) - shutil.rmtree(self.temp) + shutil.rmtree(self.temp, onerror=on_error_with_retry) def test_import_yml(self): """Verify items can be imported from a YAML file.""" @@ -521,7 +522,7 @@ def setUp(self): self.temp = tempfile.mkdtemp() def tearDown(self): - shutil.rmtree(self.temp) + shutil.rmtree(self.temp, onerror=on_error_with_retry) def test_export_yml(self): """Verify a document can be exported as a YAML file.""" @@ -577,7 +578,7 @@ def setUp(self): def tearDown(self): if os.path.exists(self.temp): - shutil.rmtree(self.temp) + shutil.rmtree(self.temp, onerror=on_error_with_retry) def test_publish_html(self): """Verify an HTML file can be created.""" diff --git a/doorstop/core/tests/test_publisher.py b/doorstop/core/tests/test_publisher.py index cdcca6893..064993d75 100644 --- a/doorstop/core/tests/test_publisher.py +++ b/doorstop/core/tests/test_publisher.py @@ -14,6 +14,7 @@ from doorstop.core import publisher from doorstop.core.builder import build from doorstop.core.tests import EMPTY, ROOT, MockDataMixIn +from doorstop.core.tests.helpers import on_error_with_retry class TestModule(MockDataMixIn, unittest.TestCase): @@ -40,7 +41,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_publish_document_unknown(self): """Verify an exception is raised when publishing unknown formats.""" diff --git a/doorstop/core/tests/test_template.py b/doorstop/core/tests/test_template.py index 937c58f70..46e335292 100644 --- a/doorstop/core/tests/test_template.py +++ b/doorstop/core/tests/test_template.py @@ -5,7 +5,6 @@ # pylint: disable=unused-argument,protected-access import os -import stat import unittest from pathlib import Path from secrets import token_hex @@ -17,7 +16,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 build_expensive_tree +from doorstop.core.tests.helpers import build_expensive_tree, on_error_with_retry class TestTemplate(MockDataMixIn, unittest.TestCase): @@ -44,10 +43,7 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): """Remove test folder.""" - rmtree( - cls.testdir, - onerror=lambda func, path, _: (os.chmod(path, stat.S_IWRITE), func(path)), - ) + rmtree(cls.testdir, onerror=on_error_with_retry) def test_standard_html_doc(self): """Verify that default html template is selected if no template is given and input is a document.""" @@ -147,6 +143,29 @@ def test_html_tree_with_custom_template(self): walk = getWalk(self.dirpath) self.assertEqual(expected_walk, walk) + def test_html_tree_with_multiple_template(self): + """Verify that a document tree with multiple custom html template throws an error.""" + # This test MUST use the expensive tree since it changes the document content + # in the source tree otherwise! + build_expensive_tree(self) + + # Check that only custom template is published. + os.makedirs(self.dirpath) + # Create a custom template folder. + doc_path = self.mock_tree.documents[0].path + os.mkdir(os.path.join(doc_path, "template")) + Path(os.path.join(doc_path, "template", "custom_css.css")).touch() + # Create another custom template folder. + doc_path = self.mock_tree.documents[1].path + os.mkdir(os.path.join(doc_path, "template")) + Path(os.path.join(doc_path, "template", "custom_css.css")).touch() + + # Act + with self.assertRaises(DoorstopError): + _, _ = template.get_template( + self.mock_tree, self.dirpath, ".html", "custom_css" + ) + def test_html_doc_with_custom_template(self): """Verify that a custom html template is used correctly.""" # This test MUST use the expensive tree since it changes the document content