From 383cde83958b89158e6d42593af62711e8074d09 Mon Sep 17 00:00:00 2001 From: Robin Stephansen Date: Mon, 15 Jan 2024 21:14:55 +0000 Subject: [PATCH 1/3] Fix to remove inactive items --- doorstop/cli/commands.py | 2 +- doorstop/cli/tests/test_all.py | 12 ++++++++++-- doorstop/core/builder.py | 11 ++++++++--- doorstop/core/document.py | 9 ++++++--- doorstop/core/tests/test_builder.py | 2 +- doorstop/core/tests/test_document.py | 9 +++++++++ doorstop/core/tests/test_tree.py | 7 +++++++ doorstop/core/tree.py | 11 ++++++++--- 8 files changed, 50 insertions(+), 13 deletions(-) diff --git a/doorstop/cli/commands.py b/doorstop/cli/commands.py index c66b73bf1..9930aee84 100644 --- a/doorstop/cli/commands.py +++ b/doorstop/cli/commands.py @@ -207,7 +207,7 @@ def run_remove(args, cwd, _, catch=True): with utilities.capture(catch=catch) as success: # get the item tree = _get_tree(args, cwd) - item = tree.find_item(args.uid) + item = tree.find_item(args.uid,only_active=False) # delete it item.delete() diff --git a/doorstop/cli/tests/test_all.py b/doorstop/cli/tests/test_all.py index adbbefe55..bf4d2a791 100644 --- a/doorstop/cli/tests/test_all.py +++ b/doorstop/cli/tests/test_all.py @@ -22,8 +22,8 @@ from doorstop.core.builder import _clear_tree from doorstop.core.document import Document -REQ_COUNT = 23 -ALL_COUNT = 55 +REQ_COUNT = 24 +ALL_COUNT = 56 class TempTestCase(unittest.TestCase): @@ -218,18 +218,26 @@ class TestRemove(unittest.TestCase): """Integration tests for the 'doorstop remove' command.""" ITEM = os.path.join(TUTORIAL, "TUT003.yml") + INACTIVE_ITEM = os.path.join(TUTORIAL, "TUT026.yml") def setUp(self): self.backup = common.read_text(self.ITEM) + self.inactive_backup = common.read_text(self.INACTIVE_ITEM) def tearDown(self): common.write_text(self.backup, self.ITEM) + common.write_text(self.inactive_backup, self.INACTIVE_ITEM) def test_remove(self): """Verify 'doorstop remove' can be called.""" self.assertIs(None, main(["remove", "tut3"])) self.assertFalse(os.path.exists(self.ITEM)) + def test_remove_inactive(self): + """Verify 'doorstop remove' can remove inactive""" + self.assertIs(None, main(["remove", "tut26"])) + self.assertFalse(os.path.exists(self.INACTIVE_ITEM)) + def test_remove_error(self): """Verify 'doorstop remove' returns an error on unknown item UIDs.""" self.assertRaises(SystemExit, main, ["remove", "tut9999"]) diff --git a/doorstop/core/builder.py b/doorstop/core/builder.py index 5669061d3..3b7cd9b88 100644 --- a/doorstop/core/builder.py +++ b/doorstop/core/builder.py @@ -94,10 +94,15 @@ def find_document(prefix): return document -def find_item(uid): - """Find an item without an explicitly building a tree.""" +def find_item(uid,only_active = True): + """Find an item without an explicitly building a tree. + + :param uid: UID + :param only_active: Returns only active items + + """ tree = _get_tree() - item = tree.find_item(uid) + item = tree.find_item(uid,only_active=only_active) return item diff --git a/doorstop/core/document.py b/doorstop/core/document.py index 9c996340a..200c9534f 100644 --- a/doorstop/core/document.py +++ b/doorstop/core/document.py @@ -562,7 +562,7 @@ def remove_item(self, value, reorder=True): """ uid = UID(value) - item = self.find_item(uid) + item = self.find_item(uid,only_active=False) item.delete() if reorder: self.reorder() @@ -780,10 +780,11 @@ def _items_by_level(items, keep=None): for item in items_at_level: yield level, item - def find_item(self, value, _kind=""): + def find_item(self, value, only_active = True, _kind=""): """Return an item by its UID. :param value: item or UID + :param only_active: Returns only active items :raises: :class:`~doorstop.common.DoorstopError` if the item cannot be found @@ -797,7 +798,9 @@ def find_item(self, value, _kind=""): if item.active: return item else: - log.trace("item is inactive: {}".format(item)) # type: ignore + log.trace("item is inactive: {}".format(item)) # type: ignore' + if not only_active: + return item raise DoorstopError("no matching{} UID: {}".format(_kind, uid)) diff --git a/doorstop/core/tests/test_builder.py b/doorstop/core/tests/test_builder.py index 9d551e282..71e530e0c 100644 --- a/doorstop/core/tests/test_builder.py +++ b/doorstop/core/tests/test_builder.py @@ -52,7 +52,7 @@ def test_find_item(self, mock_find_item): _clear_tree() uid = "req1" find_item(uid) - mock_find_item.assert_called_once_with(uid) + mock_find_item.assert_called_once_with(uid,only_active=True) def test_tree_finds_documents(self): """Verify items can be found using a convenience function.""" diff --git a/doorstop/core/tests/test_document.py b/doorstop/core/tests/test_document.py index 49506afbb..e11efb3dc 100644 --- a/doorstop/core/tests/test_document.py +++ b/doorstop/core/tests/test_document.py @@ -580,6 +580,15 @@ def test_remove_item(self, mock_remove, mock_reorder): mock_reorder.assert_called_once_with(self.document.items, keep=None, start=None) mock_remove.assert_called_once_with(item.path) + @patch("doorstop.core.document.Document._reorder_automatic") + @patch("os.remove") + def test_remove_inactive_item(self, mock_remove, mock_reorder): + """Verify an item can be removed.""" + with patch("doorstop.settings.REORDER", True): + item = self.document.remove_item("REQ005") + mock_reorder.assert_called_once_with(self.document.items, keep=None, start=None) + mock_remove.assert_called_once_with(item.path) + @patch("os.remove") def test_remove_item_contains(self, mock_remove): """Verify a removed item is not contained in the document.""" diff --git a/doorstop/core/tests/test_tree.py b/doorstop/core/tests/test_tree.py index 0af35c966..18ddc8e0f 100644 --- a/doorstop/core/tests/test_tree.py +++ b/doorstop/core/tests/test_tree.py @@ -320,6 +320,13 @@ def test_remove_item(self, mock_delete): self.tree.remove_item("req1", reorder=False) mock_delete.assert_called_once_with() + @patch("doorstop.settings.REORDER", False) + @patch("doorstop.core.item.Item.delete") + def test_remove_inactive_item(self, mock_delete): + """Verify an item can be removed from a document.""" + self.tree.remove_item("req5", reorder=False) + mock_delete.assert_called_once_with() + def test_remove_item_unknown_item(self): """Verify an exception is raised removing an unknown item.""" self.assertRaises(DoorstopError, self.tree.remove_item, "req9999") diff --git a/doorstop/core/tree.py b/doorstop/core/tree.py index 15d1fbd9d..7d1d7d32f 100644 --- a/doorstop/core/tree.py +++ b/doorstop/core/tree.py @@ -276,7 +276,7 @@ def remove_item(self, value, reorder=True): uid = UID(value) for document in self: try: - document.find_item(uid) + document.find_item(uid,only_active=False) except DoorstopError: pass # item not found in that document else: @@ -415,10 +415,11 @@ def find_document(self, value) -> Document: raise DoorstopError(Prefix.UNKNOWN_MESSAGE.format(prefix)) - def find_item(self, value, _kind=""): + def find_item(self, value, only_active=True, _kind=""): """Get an item by its UID. :param value: item or UID + :param only_active: Returns only active items :raises: :class:`~doorstop.common.DoorstopError` if the item cannot be found @@ -437,12 +438,14 @@ def find_item(self, value, _kind=""): return item else: log.trace("item is inactive: {}".format(item)) # type: ignore + if not only_active: + return item else: log.trace("found cached unknown: {}".format(uid)) # type: ignore except KeyError: for document in self: try: - item = document.find_item(uid, _kind=_kind) + item = document.find_item(uid, only_active=only_active,_kind=_kind) except DoorstopError: pass # item not found in that document else: @@ -454,6 +457,8 @@ def find_item(self, value, _kind=""): return item else: log.trace("item is inactive: {}".format(item)) # type: ignore + if not only_active: + return item log.debug("could not find item: {}".format(uid)) if settings.CACHE_ITEMS: From 94b53a2a2a58a3359cfe00ccc76d673f24632196 Mon Sep 17 00:00:00 2001 From: Robin Stephansen Date: Mon, 15 Jan 2024 21:24:05 +0000 Subject: [PATCH 2/3] Add the missing test case --- reqs/tutorial/TUT026.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 reqs/tutorial/TUT026.yml diff --git a/reqs/tutorial/TUT026.yml b/reqs/tutorial/TUT026.yml new file mode 100644 index 000000000..4cc444d11 --- /dev/null +++ b/reqs/tutorial/TUT026.yml @@ -0,0 +1,10 @@ +active: true +derived: false +header: '' +level: 5.2 +links: [] +normative: true +ref: '' +reviewed: jQyhY-PAEhuHr8zx7e2HRBUZACSJNbxvhAagi90VYw8= +text: | + An inactive requirement. From 987a8391a456f48bca6f5565b33713887a5dabb3 Mon Sep 17 00:00:00 2001 From: Robin Stephansen Date: Tue, 16 Jan 2024 21:55:56 +0000 Subject: [PATCH 3/3] Formatting fixes by black and removed rouge single quote --- doorstop/cli/commands.py | 2 +- doorstop/core/builder.py | 6 +++--- doorstop/core/document.py | 6 +++--- doorstop/core/tests/test_builder.py | 2 +- doorstop/core/tree.py | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/doorstop/cli/commands.py b/doorstop/cli/commands.py index 9930aee84..dac05c454 100644 --- a/doorstop/cli/commands.py +++ b/doorstop/cli/commands.py @@ -207,7 +207,7 @@ def run_remove(args, cwd, _, catch=True): with utilities.capture(catch=catch) as success: # get the item tree = _get_tree(args, cwd) - item = tree.find_item(args.uid,only_active=False) + item = tree.find_item(args.uid, only_active=False) # delete it item.delete() diff --git a/doorstop/core/builder.py b/doorstop/core/builder.py index 3b7cd9b88..6e25725f2 100644 --- a/doorstop/core/builder.py +++ b/doorstop/core/builder.py @@ -94,15 +94,15 @@ def find_document(prefix): return document -def find_item(uid,only_active = True): +def find_item(uid, only_active=True): """Find an item without an explicitly building a tree. - + :param uid: UID :param only_active: Returns only active items """ tree = _get_tree() - item = tree.find_item(uid,only_active=only_active) + item = tree.find_item(uid, only_active=only_active) return item diff --git a/doorstop/core/document.py b/doorstop/core/document.py index 200c9534f..ef620faa7 100644 --- a/doorstop/core/document.py +++ b/doorstop/core/document.py @@ -562,7 +562,7 @@ def remove_item(self, value, reorder=True): """ uid = UID(value) - item = self.find_item(uid,only_active=False) + item = self.find_item(uid, only_active=False) item.delete() if reorder: self.reorder() @@ -780,7 +780,7 @@ def _items_by_level(items, keep=None): for item in items_at_level: yield level, item - def find_item(self, value, only_active = True, _kind=""): + def find_item(self, value, only_active=True, _kind=""): """Return an item by its UID. :param value: item or UID @@ -798,7 +798,7 @@ def find_item(self, value, only_active = True, _kind=""): if item.active: return item else: - log.trace("item is inactive: {}".format(item)) # type: ignore' + log.trace("item is inactive: {}".format(item)) # type: ignore if not only_active: return item diff --git a/doorstop/core/tests/test_builder.py b/doorstop/core/tests/test_builder.py index 71e530e0c..ec4856c12 100644 --- a/doorstop/core/tests/test_builder.py +++ b/doorstop/core/tests/test_builder.py @@ -52,7 +52,7 @@ def test_find_item(self, mock_find_item): _clear_tree() uid = "req1" find_item(uid) - mock_find_item.assert_called_once_with(uid,only_active=True) + mock_find_item.assert_called_once_with(uid, only_active=True) def test_tree_finds_documents(self): """Verify items can be found using a convenience function.""" diff --git a/doorstop/core/tree.py b/doorstop/core/tree.py index 7d1d7d32f..4bb14afe4 100644 --- a/doorstop/core/tree.py +++ b/doorstop/core/tree.py @@ -276,7 +276,7 @@ def remove_item(self, value, reorder=True): uid = UID(value) for document in self: try: - document.find_item(uid,only_active=False) + document.find_item(uid, only_active=False) except DoorstopError: pass # item not found in that document else: @@ -445,7 +445,7 @@ def find_item(self, value, only_active=True, _kind=""): except KeyError: for document in self: try: - item = document.find_item(uid, only_active=only_active,_kind=_kind) + item = document.find_item(uid, only_active=only_active, _kind=_kind) except DoorstopError: pass # item not found in that document else: