From dbf55daa876c3a3bf1ed20b6d82dc76f8e5c16e6 Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Mon, 13 Jul 2020 19:00:30 -0700 Subject: [PATCH] Add module to default rule names. (#10341) ### Problem See #10296. ### Result Fixes #10296. [ci skip-rust-tests] --- .../pants/engine/internals/engine_test.py | 72 ++++++++++++++----- src/python/pants/engine/rules.py | 6 +- src/python/pants/engine/rules_test.py | 2 +- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/python/pants/engine/internals/engine_test.py b/src/python/pants/engine/internals/engine_test.py index c76277824c6..5d94a6c83b7 100644 --- a/src/python/pants/engine/internals/engine_test.py +++ b/src/python/pants/engine/internals/engine_test.py @@ -109,7 +109,7 @@ class Epsilon: pass -@rule(canonical_name="rule_one", desc="Rule number 1", level=LogLevel.INFO) +@rule(canonical_name="canonical_rule_one", desc="Rule number 1", level=LogLevel.INFO) async def rule_one_function(i: Input) -> Beta: """This rule should be the first one executed by the engine, and thus have no parent.""" a = Alpha() @@ -411,24 +411,38 @@ def test_streaming_workunits_parent_id_and_rule_metadata(self): # rule_one should complete well-after the other rules because of the artificial delay in it caused by the sleep(). assert {item["name"] for item in tracker.finished_workunit_chunks[0]} == { - "rule_two", - "rule_three", - "rule_four", + "pants.engine.internals.engine_test.rule_two", + "pants.engine.internals.engine_test.rule_three", + "pants.engine.internals.engine_test.rule_four", } # Because of the artificial delay in rule_one, it should have time to be reported as # started but not yet finished. started = list(itertools.chain.from_iterable(tracker.started_workunit_chunks)) - assert len(list(item for item in started if item["name"] == "rule_one")) > 0 + assert len(list(item for item in started if item["name"] == "canonical_rule_one")) > 0 - assert {item["name"] for item in tracker.finished_workunit_chunks[1]} == {"rule_one"} + assert {item["name"] for item in tracker.finished_workunit_chunks[1]} == { + "canonical_rule_one" + } finished = list(itertools.chain.from_iterable(tracker.finished_workunit_chunks)) - r1 = next(item for item in finished if item["name"] == "rule_one") - r2 = next(item for item in finished if item["name"] == "rule_two") - r3 = next(item for item in finished if item["name"] == "rule_three") - r4 = next(item for item in finished if item["name"] == "rule_four") + r1 = next(item for item in finished if item["name"] == "canonical_rule_one") + r2 = next( + item + for item in finished + if item["name"] == "pants.engine.internals.engine_test.rule_two" + ) + r3 = next( + item + for item in finished + if item["name"] == "pants.engine.internals.engine_test.rule_three" + ) + r4 = next( + item + for item in finished + if item["name"] == "pants.engine.internals.engine_test.rule_four" + ) # rule_one should have no parent_id because its actual parent workunit was filted based on level assert r1.get("parent_id", None) is None @@ -465,12 +479,18 @@ def test_streaming_workunit_log_levels(self) -> None: select = next( item for item in finished - if item["name"] not in {"rule_one", "rule_two", "rule_three", "rule_four"} + if item["name"] + not in { + "canonical_rule_one", + "pants.engine.internals.engine_test.rule_two", + "pants.engine.internals.engine_test.rule_three", + "pants.engine.internals.engine_test.rule_four", + } ) assert select["name"] == "select" assert select["level"] == "DEBUG" - r1 = next(item for item in finished if item["name"] == "rule_one") + r1 = next(item for item in finished if item["name"] == "canonical_rule_one") assert r1["parent_id"] == select["span_id"] def test_streaming_workunit_log_level_parent_rewrite(self) -> None: @@ -494,8 +514,12 @@ def test_streaming_workunit_log_level_parent_rewrite(self) -> None: finished = list(itertools.chain.from_iterable(tracker.finished_workunit_chunks)) assert len(finished) == 2 - r_A = next(item for item in finished if item["name"] == "rule_A") - r_C = next(item for item in finished if item["name"] == "rule_C") + r_A = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.rule_A" + ) + r_C = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.rule_C" + ) assert "parent_id" not in r_A assert r_C["parent_id"] == r_A["span_id"] @@ -517,9 +541,15 @@ def test_streaming_workunit_log_level_parent_rewrite(self) -> None: assert tracker.finished finished = list(itertools.chain.from_iterable(tracker.finished_workunit_chunks)) - r_A = next(item for item in finished if item["name"] == "rule_A") - r_B = next(item for item in finished if item["name"] == "rule_B") - r_C = next(item for item in finished if item["name"] == "rule_C") + r_A = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.rule_A" + ) + r_B = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.rule_B" + ) + r_C = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.rule_C" + ) assert r_B["parent_id"] == r_A["span_id"] assert r_C["parent_id"] == r_B["span_id"] @@ -552,7 +582,9 @@ def a_rule(n: int) -> ModifiedOutput: scheduler.product_request(ModifiedOutput, subjects=[0]) finished = list(itertools.chain.from_iterable(tracker.finished_workunit_chunks)) - workunit = next(item for item in finished if item["name"] == "a_rule") + workunit = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.a_rule" + ) assert workunit["level"] == "ERROR" def test_engine_aware_none_case(self): @@ -586,7 +618,9 @@ def a_rule(n: int) -> ModifiedOutput: scheduler.product_request(ModifiedOutput, subjects=[0]) finished = list(itertools.chain.from_iterable(tracker.finished_workunit_chunks)) - workunit = next(item for item in finished if item["name"] == "a_rule") + workunit = next( + item for item in finished if item["name"] == "pants.engine.internals.engine_test.a_rule" + ) assert workunit["level"] == "DEBUG" def test_artifacts_on_engine_aware_type(self) -> None: diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 9c95b2d023b..2b4fd2f2e9f 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -325,11 +325,11 @@ def rule_decorator(func, **kwargs) -> Callable: validate_parameter_types(func_id, parameter_types, cacheable) # Set a default canonical name if one is not explicitly provided. For Goal classes - # this is the name of the Goal; for other named ruled this is the __name__ of the function - # that implements it. + # this is the name of the Goal; for other named ruled this is the module and name of the + # function that implements it. effective_name = kwargs.get("canonical_name") if effective_name is None: - effective_name = return_type.name if is_goal_cls else func.__name__ + effective_name = return_type.name if is_goal_cls else f"{func.__module__}.{func.__name__}" effective_desc = kwargs.get("desc") if effective_desc is None and is_goal_cls: diff --git a/src/python/pants/engine/rules_test.py b/src/python/pants/engine/rules_test.py index 7a569e34176..2129ef7d0e8 100644 --- a/src/python/pants/engine/rules_test.py +++ b/src/python/pants/engine/rules_test.py @@ -403,7 +403,7 @@ def a_named_rule(a: int, b: str) -> bool: return False self.assertIsNotNone(a_named_rule.rule) - self.assertEqual(a_named_rule.rule.canonical_name, "a_named_rule") + self.assertEqual(a_named_rule.rule.canonical_name, "pants.engine.rules_test.a_named_rule") self.assertEqual(a_named_rule.rule.desc, None) self.assertEqual(a_named_rule.rule.level, LogLevel.INFO)