Fix macro modified from previous state with pkg (dbt-labs#5224)

* Fix macro modified from previous state with pkg

When iterating through nodes to check if any of its macro dependencies
have been modified, the state selector will first check all upstream
macro dependencies before returning a judgement.

.changes/unreleased/Fixes-20220509-131312.yaml

@@ -0,0 +1,8 @@
+kind: Fixes
+body: Changed how `--select state:modified` detects changes for macros nodes depend
+  on
+time: 2022-05-09T13:13:12.889074-05:00
+custom:
+  Author: stu-k
+  Issue: "5202"
+  PR: "5224"

core/dbt/graph/selector_methods.py

@@ -425,25 +425,31 @@ def _macros_modified(self) -> List[str]:
         return modified
 
     def recursively_check_macros_modified(self, node, visited_macros):
-        # loop through all macros that this node depends on
         for macro_uid in node.depends_on.macros:
-            # avoid infinite recursion if we've already seen this macro
             if macro_uid in visited_macros:
                 continue
             visited_macros.append(macro_uid)
-            # is this macro one of the modified macros?
+
             if macro_uid in self.modified_macros:
                 return True
-            # if not, and this macro depends on other macros, keep looping
+
+            # this macro hasn't been modified, but depends on other
+            # macros which each need to be tested for modification
             macro_node = self.manifest.macros[macro_uid]
             if len(macro_node.depends_on.macros) > 0:
-                return self.recursively_check_macros_modified(macro_node, visited_macros)
+                upstream_macros_changed = self.recursively_check_macros_modified(
+                    macro_node, visited_macros
+                )
+                if upstream_macros_changed:
+                    return True
+                continue
+
             # this macro hasn't been modified, but we haven't checked
             # the other macros the node depends on, so keep looking
-            elif len(node.depends_on.macros) > len(visited_macros):
+            if len(node.depends_on.macros) > len(visited_macros):
                 continue
-            else:
-                return False
+
+        return False
 
     def check_macros_modified(self, node):
         # check if there are any changes in macros the first time

test/unit/test_graph_selector_methods.py

@@ -1087,3 +1087,38 @@ def test_select_state_changed_test_macros(manifest, previous_state):
     assert search_manifest_using_method(
         manifest, method, 'modified.macros') == {'model1', 'model2'}
     assert not search_manifest_using_method(manifest, method, 'new')
+
+def test_select_state_changed_test_macros_with_upstream_change(manifest, previous_state):
+    changed_macro = make_macro('dbt', 'changed_macro', 'blablabla')
+    add_macro(manifest, changed_macro)
+    add_macro(previous_state.manifest, changed_macro.replace(macro_sql='something different'))
+
+    unchanged_macro1 = make_macro('dbt', 'unchanged_macro', 'blablabla')
+    add_macro(manifest, unchanged_macro1)
+    add_macro(previous_state.manifest, unchanged_macro1)
+
+    unchanged_macro2 = make_macro('dbt', 'unchanged_macro', 'blablabla', depends_on_macros=[unchanged_macro1.unique_id, changed_macro.unique_id])
+    add_macro(manifest, unchanged_macro2)
+    add_macro(previous_state.manifest, unchanged_macro2)
+
+    unchanged_macro3 = make_macro('dbt', 'unchanged_macro', 'blablabla', depends_on_macros=[changed_macro.unique_id, unchanged_macro1.unique_id])
+    add_macro(manifest, unchanged_macro3)
+    add_macro(previous_state.manifest, unchanged_macro3)
+
+    model1 = make_model('dbt', 'model1', 'blablabla',
+            depends_on_macros=[unchanged_macro1.unique_id])
+    add_node(manifest, model1)
+    add_node(previous_state.manifest, model1)
+
+    model2 = make_model('dbt', 'model2', 'blablabla',
+            depends_on_macros=[unchanged_macro3.unique_id])
+    add_node(manifest, model2)
+    add_node(previous_state.manifest, model2)
+
+    method = statemethod(manifest, previous_state)
+
+    assert search_manifest_using_method(
+        manifest, method, 'modified') == {'model1', 'model2'}
+    assert search_manifest_using_method(
+        manifest, method, 'modified.macros') == {'model1', 'model2'}
+    assert not search_manifest_using_method(manifest, method, 'new')