Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template fixes #629

Merged
merged 25 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3c57c44
Fix #628, part 1. Missed check of "doc" in attributes_default.
neerdoc Feb 12, 2024
a545752
Added double template check and also allow any document to define the…
neerdoc Feb 12, 2024
184cc4e
Set template to None as default.
neerdoc Feb 12, 2024
9eccfb7
Ignore .python-version
neerdoc Feb 12, 2024
3da2701
Added fix for intermittent failing Windows tests due to race in folde…
neerdoc Feb 14, 2024
8c92174
Merge remote-tracking branch 'origin/develop' into template_fixes
neerdoc Feb 14, 2024
7500243
Fixed make test-cover command.
neerdoc Feb 14, 2024
880b910
Added on_error_retry function to try to handle Windows testing.
neerdoc Feb 14, 2024
51be0cd
Updated tests rmtree to use the on_error_retry function.
neerdoc Feb 14, 2024
9a04c27
Added hook for stopping pushes of code without test coverage.
neerdoc Feb 14, 2024
158515f
Changed hook.
neerdoc Feb 14, 2024
880f777
Removed check from pre-commit hooks as this is not standard git.
neerdoc Feb 14, 2024
5e304af
Added a test to check for change coverage.
neerdoc Feb 14, 2024
874c7de
Moved coverage test to only run on linux as this is enough.
neerdoc Feb 14, 2024
fdde19e
Fixed rounding issue. 3.10 -> 3.1....
neerdoc Feb 14, 2024
e7436aa
Cleanup call tree and made the test fail.
neerdoc Feb 14, 2024
3c741d3
Not possible with shell expansion in gitlab actions.
neerdoc Feb 14, 2024
3be95d1
Need to run normal test first.
neerdoc Feb 14, 2024
8bd265c
Print out the develop commit hash.
neerdoc Feb 14, 2024
bf6a45c
Debugging the CI-script.
neerdoc Feb 14, 2024
070afbc
Trying another way of comparing with develop.
neerdoc Feb 14, 2024
c3bcd94
Test original way but after pull.
neerdoc Feb 14, 2024
b125387
Deeper fetch needed. Also updated action versions.
neerdoc Feb 15, 2024
388d97d
Git fetch still needed.
neerdoc Feb 15, 2024
f258ae8
Added missing test.
neerdoc Feb 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .github/workflows/change-coverage.yml
Original file line number Diff line number Diff line change
@@ -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 ' ')
8 changes: 4 additions & 4 deletions .github/workflows/execute-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand All @@ -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
Expand All @@ -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') }}
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Icon*
# Temporary virtual environment files
/.cache/
/.venv/
/.python-version

# Temporary server files
.env
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions doorstop/cli/tests/test_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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."""
Expand Down
33 changes: 17 additions & 16 deletions doorstop/core/publishers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion doorstop/core/publishers/tests/test_publisher_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,6 +23,7 @@
MockDocument,
MockItem,
)
from doorstop.core.tests.helpers import on_error_with_retry
from doorstop.core.types import UID


Expand All @@ -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")
Expand Down
4 changes: 3 additions & 1 deletion doorstop/core/publishers/tests/test_publisher_html_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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."""
Expand Down
4 changes: 3 additions & 1 deletion doorstop/core/publishers/tests/test_publisher_latex_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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."""
Expand Down
4 changes: 3 additions & 1 deletion doorstop/core/publishers/tests/test_publisher_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,6 +21,7 @@
MockItem,
MockItemAndVCS,
)
from doorstop.core.tests.helpers import on_error_with_retry


class TestModule(MockDataMixIn, unittest.TestCase):
Expand All @@ -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."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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."""
Expand Down
4 changes: 3 additions & 1 deletion doorstop/core/publishers/tests/test_publisher_text.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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."""
Expand Down
12 changes: 11 additions & 1 deletion doorstop/core/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 29 additions & 0 deletions doorstop/core/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.")
Loading
Loading