From: Jacob Lifshay Date: Wed, 6 Jul 2022 03:21:58 +0000 (-0700) Subject: clean up after changing Node.is_in_nlnet_mou to be a computed property X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=589c66b129006099e7279bc08b7cc79e8caf5a81;p=utils.git clean up after changing Node.is_in_nlnet_mou to be a computed property --- diff --git a/src/budget_sync/budget_graph.py b/src/budget_sync/budget_graph.py index cf4fbc0..b3d457c 100644 --- a/src/budget_sync/budget_graph.py +++ b/src/budget_sync/budget_graph.py @@ -314,7 +314,6 @@ class Node: fixed_budget_excluding_subtasks: Money fixed_budget_including_subtasks: Money milestone_str: Optional[str] - is_in_nlnet_mou: bool def __init__(self, graph: "BudgetGraph", bug: Bug): self.graph = graph @@ -454,13 +453,16 @@ class Node: @cached_property def is_in_nlnet_mou(self): - """returns true if this bugreport is a child of the top-level milestone. - it does *not* return true for the top-level bugreport itself because - only the immediate child-nodes comprise the MoU. + """returns true if this bugreport is an immediate child of a top-level + milestone. it does *not* return true for the top-level bug itself + because only the immediate children comprise the MoU. """ - if self.parent is None: - return False - return self.parent == self.root + try: + if self.parent is not None and self.milestone is not None: + return self.parent.bug.id == self.milestone.canonical_bug_id + except BudgetGraphBaseError: + pass + return False @cached_property def closest_bug_in_mou(self) -> Optional["Node"]: @@ -676,36 +678,6 @@ class BudgetGraphIncorrectRootForMilestone(BudgetGraphError): f"#{self.milestone_canonical_bug_id}") -class BudgetGraphRootWithMilestoneNotInMoU(BudgetGraphError): - def __init__(self, bug_id: int, milestone: str): - super().__init__(bug_id, bug_id) - self.milestone = milestone - - def __str__(self): - return (f"Bug #{self.bug_id} has no parent bug set and has an " - f"assigned milestone {self.milestone!r} but isn't set " - f"to be part of the signed MoU") - - -class BudgetGraphInMoUButParentNotInMoU(BudgetGraphError): - def __init__(self, bug_id: int, parent_bug_id: int, root_bug_id: int, - milestone: str): - super().__init__(bug_id, root_bug_id) - self.parent_bug_id = parent_bug_id - self.milestone = milestone - - def __str__(self): - return (f"Bug #{self.bug_id} is set to be part of the signed MoU for " - f"milestone {self.milestone!r}, but its parent bug isn't set " - f"to be part of the signed MoU") - - -class BudgetGraphInMoUWithoutMilestone(BudgetGraphError): - def __str__(self): - return (f"Bug #{self.bug_id} is set to be part of a signed MoU but " - f"has no milestone set") - - class BudgetGraph: nodes: Dict[int, Node] @@ -751,10 +723,6 @@ class BudgetGraph: node.bug.id, node.milestone.identifier, node.milestone.canonical_bug_id )) - # the root level bugs are not themselves "the child MoU list" - #elif not node.is_in_nlnet_mou: - # errors.append(BudgetGraphRootWithMilestoneNotInMoU( - # node.bug.id, node.milestone_str)) except BudgetGraphBaseError as e: errors.append(e) @@ -774,17 +742,6 @@ class BudgetGraph: errors.append(BudgetGraphMilestoneMismatch( node.bug.id, root.bug.id)) - if node.is_in_nlnet_mou: - if node.milestone_str is None: - errors.append(BudgetGraphInMoUWithoutMilestone(node.bug.id, - root.bug.id)) - # don't consider the top-level root to be part of the MoU - #elif node.parent is not None and \ - # not node.parent.is_in_nlnet_mou: - # errors.append(BudgetGraphInMoUButParentNotInMoU( - # node.bug.id, node.parent.bug.id, root.bug.id, - # node.milestone_str)) - if node.budget_excluding_subtasks < 0 \ or node.budget_including_subtasks < 0: errors.append(BudgetGraphNegativeMoney( diff --git a/src/budget_sync/test/mock_bug.py b/src/budget_sync/test/mock_bug.py index 4f926fc..17be594 100644 --- a/src/budget_sync/test/mock_bug.py +++ b/src/budget_sync/test/mock_bug.py @@ -12,8 +12,7 @@ class MockBug: cf_payees_list: str = "", summary: str = "", status: Union[str, BugStatus] = BugStatus.CONFIRMED, - assigned_to: str = "user@example.com", - cf_is_in_nlnet_mou2: str = ""): + assigned_to: str = "user@example.com"): self.id = bug_id self.__budget_parent = cf_budget_parent self.cf_budget = cf_budget @@ -25,7 +24,6 @@ class MockBug: self.summary = summary self.status = str(status) self.assigned_to = assigned_to - self.cf_is_in_nlnet_mou2 = cf_is_in_nlnet_mou2 @property def cf_budget_parent(self) -> int: diff --git a/src/budget_sync/test/test_budget_graph.py b/src/budget_sync/test/test_budget_graph.py index b0f9494..1221946 100644 --- a/src/budget_sync/test/test_budget_graph.py +++ b/src/budget_sync/test/test_budget_graph.py @@ -8,9 +8,7 @@ from budget_sync.budget_graph import ( BudgetGraphNegativePayeeMoney, BudgetGraphPayeesParseError, BudgetGraphPayeesMoneyMismatch, BudgetGraphUnknownMilestone, BudgetGraphIncorrectRootForMilestone, - BudgetGraphUnknownStatus, BudgetGraphUnknownAssignee, - BudgetGraphRootWithMilestoneNotInMoU, BudgetGraphInMoUButParentNotInMoU, - BudgetGraphInMoUWithoutMilestone) + BudgetGraphUnknownStatus, BudgetGraphUnknownAssignee) from budget_sync.money import Money from budget_sync.util import BugStatus from typing import List, Type @@ -96,25 +94,6 @@ class TestErrorFormatting(unittest.TestCase): "Total budget assigned to payees (cf_payees_list) doesn't match " "expected value: bug #1, calculated total 123, expected value 456") - def test_budget_graph_root_with_milestone_not_in_mou(self): - self.assertEqual(str( - BudgetGraphRootWithMilestoneNotInMoU(1, "milestone 1")), - "Bug #1 has no parent bug set and has an assigned milestone " - "'milestone 1' but isn't set to be part of the signed MoU") - - def test_budget_graph_in_mou_but_parent_not_in_mou(self): - self.assertEqual(str( - BudgetGraphInMoUButParentNotInMoU(5, 3, 1, "milestone 1")), - "Bug #5 is set to be part of the signed MoU for milestone " - "'milestone 1', but its parent bug isn't set to be part of " - "the signed MoU") - - def test_budget_graph_in_mou_without_milestone(self): - self.assertEqual(str( - BudgetGraphInMoUWithoutMilestone(1, 5)), - "Bug #1 is set to be part of a signed MoU but has no " - "milestone set") - EXAMPLE_BUG1 = MockBug(bug_id=1, cf_budget_parent=None, @@ -150,8 +129,7 @@ EXAMPLE_PARENT_BUG1 = MockBug(bug_id=1, cf_total_budget="20", cf_nlnet_milestone="milestone 1", cf_payees_list="", - summary="", - cf_is_in_nlnet_mou2="Yes") + summary="") EXAMPLE_CHILD_BUG2 = MockBug(bug_id=2, cf_budget_parent=1, cf_budget="10", @@ -208,7 +186,7 @@ class TestBudgetGraph(unittest.TestCase): "budget_excluding_subtasks=10, budget_including_subtasks=20, " "fixed_budget_excluding_subtasks=10, " "fixed_budget_including_subtasks=20, milestone_str='milestone " - "1', is_in_nlnet_mou=True, " + "1', is_in_nlnet_mou=False, " "milestone=Milestone(config=..., identifier='milestone 1', " "canonical_bug_id=1), immediate_children=[#2], payments=[], " "status=BugStatus.CONFIRMED, assignee=Person<'person3'>, " @@ -217,7 +195,7 @@ class TestBudgetGraph(unittest.TestCase): "budget_including_subtasks=10, " "fixed_budget_excluding_subtasks=10, " "fixed_budget_including_subtasks=10, milestone_str='milestone " - "1', is_in_nlnet_mou=False, " + "1', is_in_nlnet_mou=True, " "milestone=Milestone(config=..., identifier='milestone 1', " "canonical_bug_id=1), immediate_children=[], payments=[], " "status=BugStatus.CONFIRMED, assignee=Person<'person3'>, " @@ -591,8 +569,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget=total_budget, cf_nlnet_milestone="milestone 1", cf_payees_list=payees_list, - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), MockBug(bug_id=2, cf_budget_parent=1, cf_budget=child_budget, @@ -614,8 +591,7 @@ alias2 = {paid=2020-03-16,amount=23} node1.fixed_budget_including_subtasks), cf_nlnet_milestone="milestone 1", cf_payees_list=payees_list, - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), MockBug(bug_id=2, cf_budget_parent=1, cf_budget=child_budget, @@ -915,8 +891,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="-10", cf_nlnet_milestone="milestone 1", cf_payees_list="", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, [ @@ -934,8 +909,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="0", cf_nlnet_milestone="milestone 1", cf_payees_list="", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, [ @@ -953,8 +927,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="-10", cf_nlnet_milestone="milestone 1", cf_payees_list="", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -970,8 +943,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="0", cf_nlnet_milestone="milestone 1", cf_payees_list=cf_payees_list, - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) self.assertErrorTypesMatches(bg.get_errors(), error_types) self.assertEqual(len(bg.nodes), 1) @@ -1199,8 +1171,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="10", cf_nlnet_milestone="milestone 1", cf_payees_list="person1 = 5\nperson2 = 10", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -1218,8 +1189,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="0", cf_nlnet_milestone="milestone 1", cf_payees_list=cf_payees_list, - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG).get_errors() self.assertErrorTypesMatches(errors, [BudgetGraphPayeesParseError]) @@ -1315,8 +1285,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="10", cf_nlnet_milestone="milestone 1", cf_payees_list="""person1 = -10""", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -1340,8 +1309,7 @@ alias2 = {paid=2020-03-16,amount=23} person1 = 5 alias1 = 5 """, - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, []) @@ -1383,8 +1351,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="10", cf_nlnet_milestone="milestone 2", cf_payees_list="", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -1400,8 +1367,7 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="0", cf_nlnet_milestone="milestone 2", cf_payees_list="", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) errors = bg.get_errors() self.assertErrorTypesMatches(errors, []) @@ -1414,16 +1380,14 @@ alias2 = {paid=2020-03-16,amount=23} cf_total_budget="10", cf_nlnet_milestone="milestone 1", cf_payees_list="person1 = 3\nperson2 = 7", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), MockBug(bug_id=2, cf_budget_parent=None, cf_budget="10", cf_total_budget="10", cf_nlnet_milestone="milestone 2", cf_payees_list="person3 = 5\nperson2 = 5", - summary="", - cf_is_in_nlnet_mou2="Yes"), + summary=""), ], EXAMPLE_CONFIG) self.assertErrorTypesMatches(bg.get_errors(), []) person1 = EXAMPLE_CONFIG.people["person1"] @@ -1484,65 +1448,31 @@ alias2 = {paid=2020-03-16,amount=23} def test_closest_bug_in_mou(self): bg = BudgetGraph([ - MockBug(bug_id=1, cf_nlnet_milestone="milestone 1", - cf_is_in_nlnet_mou2="Yes"), + MockBug(bug_id=1, cf_nlnet_milestone="milestone 1"), MockBug(bug_id=2, cf_budget_parent=1, - cf_nlnet_milestone="milestone 1", - cf_is_in_nlnet_mou2="Yes"), + cf_nlnet_milestone="milestone 1"), MockBug(bug_id=3, cf_budget_parent=2, - cf_nlnet_milestone="milestone 1", - cf_is_in_nlnet_mou2="Yes"), - MockBug(bug_id=4, cf_budget_parent=2, + cf_nlnet_milestone="milestone 1"), + MockBug(bug_id=4, cf_budget_parent=1, cf_nlnet_milestone="milestone 1"), MockBug(bug_id=5, cf_budget_parent=4, cf_nlnet_milestone="milestone 1"), MockBug(bug_id=6), + MockBug(bug_id=7, cf_nlnet_milestone="bad milestone"), + MockBug(bug_id=8, cf_budget_parent=7, + cf_nlnet_milestone="bad milestone"), ], EXAMPLE_CONFIG) errors = bg.get_errors() - self.assertErrorTypesMatches(errors, []) - self.assertEqual(bg.nodes[1].closest_bug_in_mou, bg.nodes[1]) + self.assertErrorTypesMatches(errors, [BudgetGraphUnknownMilestone, + BudgetGraphUnknownMilestone]) + self.assertEqual(bg.nodes[1].closest_bug_in_mou, None) self.assertEqual(bg.nodes[2].closest_bug_in_mou, bg.nodes[2]) - self.assertEqual(bg.nodes[3].closest_bug_in_mou, bg.nodes[3]) - self.assertEqual(bg.nodes[4].closest_bug_in_mou, bg.nodes[2]) - self.assertEqual(bg.nodes[5].closest_bug_in_mou, bg.nodes[2]) + self.assertEqual(bg.nodes[3].closest_bug_in_mou, bg.nodes[2]) + self.assertEqual(bg.nodes[4].closest_bug_in_mou, bg.nodes[4]) + self.assertEqual(bg.nodes[5].closest_bug_in_mou, bg.nodes[4]) self.assertEqual(bg.nodes[6].closest_bug_in_mou, None) - - def test_root_with_milestone_not_in_mou(self): - bg = BudgetGraph([ - MockBug(bug_id=1, cf_nlnet_milestone="milestone 1"), - ], EXAMPLE_CONFIG) - errors = bg.get_errors() - self.assertErrorTypesMatches(errors, - [BudgetGraphRootWithMilestoneNotInMoU]) - self.assertEqual(errors[0].bug_id, 1) - self.assertEqual(errors[0].root_bug_id, 1) - self.assertEqual(errors[0].milestone, "milestone 1") - - def test_budget_graph_in_mou_without_milestone(self): - bg = BudgetGraph([ - MockBug(bug_id=1, cf_is_in_nlnet_mou2="Yes"), - ], EXAMPLE_CONFIG) - errors = bg.get_errors() - self.assertErrorTypesMatches(errors, - [BudgetGraphInMoUWithoutMilestone]) - self.assertEqual(errors[0].bug_id, 1) - self.assertEqual(errors[0].root_bug_id, 1) - - def test_in_mou_but_parent_not_in_mou(self): - bg = BudgetGraph([ - MockBug(bug_id=1, cf_nlnet_milestone="milestone 1", - cf_is_in_nlnet_mou2="Yes"), - MockBug(bug_id=2, cf_nlnet_milestone="milestone 1", - cf_budget_parent=1), - MockBug(bug_id=3, cf_nlnet_milestone="milestone 1", - cf_budget_parent=2, cf_is_in_nlnet_mou2="Yes"), - ], EXAMPLE_CONFIG) - errors = bg.get_errors() - self.assertErrorTypesMatches(errors, - [BudgetGraphInMoUButParentNotInMoU]) - self.assertEqual(errors[0].bug_id, 3) - self.assertEqual(errors[0].root_bug_id, 1) - self.assertEqual(errors[0].parent_bug_id, 2) + self.assertEqual(bg.nodes[7].closest_bug_in_mou, None) + self.assertEqual(bg.nodes[8].closest_bug_in_mou, None) if __name__ == "__main__": diff --git a/src/budget_sync/test/test_write_budget_csv.py b/src/budget_sync/test/test_write_budget_csv.py index 08182fd..a95a686 100644 --- a/src/budget_sync/test/test_write_budget_csv.py +++ b/src/budget_sync/test/test_write_budget_csv.py @@ -41,8 +41,7 @@ class TestWriteBudgetMarkdown(unittest.TestCase): person2 = {amount=421,paid=2020-01-01} """, summary="", - assigned_to="person2@example.com", - cf_is_in_nlnet_mou2="Yes"), + assigned_to="person2@example.com"), MockBug(bug_id=2, cf_budget_parent=None, cf_budget="0", @@ -50,8 +49,7 @@ class TestWriteBudgetMarkdown(unittest.TestCase): cf_nlnet_milestone="milestone 2", cf_payees_list="", summary="", - assigned_to="person2@example.com", - cf_is_in_nlnet_mou2="Yes"), + assigned_to="person2@example.com"), ], config) self.assertEqual([], budget_graph.get_errors()) # pretty_print(budget_graph) diff --git a/src/budget_sync/test/test_write_budget_markdown.py b/src/budget_sync/test/test_write_budget_markdown.py index 0a8b5dc..1698767 100644 --- a/src/budget_sync/test/test_write_budget_markdown.py +++ b/src/budget_sync/test/test_write_budget_markdown.py @@ -122,8 +122,7 @@ class TestWriteBudgetMarkdown(unittest.TestCase): cf_nlnet_milestone="milestone 1", cf_payees_list="", summary="", - assigned_to="person1@example.com", - cf_is_in_nlnet_mou2="Yes"), + assigned_to="person1@example.com"), MockBug(bug_id=2, cf_budget_parent=1, cf_budget="100", @@ -131,8 +130,7 @@ class TestWriteBudgetMarkdown(unittest.TestCase): cf_nlnet_milestone="milestone 1", cf_payees_list="person2 = 100", summary="", - assigned_to="person1@example.com", - cf_is_in_nlnet_mou2="Yes"), + assigned_to="person1@example.com"), MockBug(bug_id=3, cf_budget_parent=2, cf_budget="100", @@ -175,7 +173,7 @@ class TestWriteBudgetMarkdown(unittest.TestCase): b'* [Bug #3](https://bugzilla.example.com/show_bug.cgi?id=3):\n' b' \n' b' * €100 which is the total amount\n' - b' * the closest parent task which is in the MoU is\n' + b' * this task is part of MoU Milestone\n' b' [Bug #2](https://bugzilla.example.com/show_bug.cgi?id=2)\n' ), '/output_dir/person2.mdwn': ( @@ -196,11 +194,11 @@ class TestWriteBudgetMarkdown(unittest.TestCase): b'* [Bug #2](https://bugzilla.example.com/show_bug.cgi?id=2):\n' b' \n' b' * €100 which is the total amount\n' - b' * this task is in the MoU\n' + b' * this task is a MoU Milestone\n' b'* [Bug #4](https://bugzilla.example.com/show_bug.cgi?id=4):\n' b' \n' b' * €100 which is the total amount\n' - b' * the closest parent task which is in the MoU is\n' + b' * this task is part of MoU Milestone\n' b' [Bug #2](https://bugzilla.example.com/show_bug.cgi?id=2)\n' ), }, filesystem) diff --git a/src/budget_sync/write_budget_markdown.py b/src/budget_sync/write_budget_markdown.py index 757bdac..427f97f 100644 --- a/src/budget_sync/write_budget_markdown.py +++ b/src/budget_sync/write_budget_markdown.py @@ -76,18 +76,6 @@ class MarkdownWriter: summary = markdown_escape(node.bug.summary) print(f"* [Bug #{node.bug.id}]({node.bug_url}):\n {summary}", file=self.buffer) - closest = node.closest_bug_in_mou - if closest is node: - print(f" * this task is an MoU Milestone", - file=self.buffer) - elif closest is not None: - print(f" * this task is part of MoU milestone\n" - f" [Bug #{closest.bug.id}]({closest.bug_url})", - file=self.buffer) - elif payment is not None: # only report this if there's a payment - print(f" * neither this task nor any parent tasks are in " - f"the MoU", - file=self.buffer) if payment is not None: if node.fixed_budget_excluding_subtasks \ != node.budget_excluding_subtasks: @@ -109,6 +97,18 @@ class MarkdownWriter: else: print(f" * €{payment.amount} which is the total amount", file=self.buffer) + closest = node.closest_bug_in_mou + if closest is node: + print(f" * this task is a MoU Milestone", + file=self.buffer) + elif closest is not None: + print(f" * this task is part of MoU Milestone\n" + f" [Bug #{closest.bug.id}]({closest.bug_url})", + file=self.buffer) + elif payment is not None: # only report this if there's a payment + print(f" * neither this task nor any parent tasks are in " + f"the MoU", + file=self.buffer) def _markdown_for_person(person: Person,