Fix macro modified from previous state with pkg

.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

@@ -416,25 +416,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

@@ -1071,3 +1071,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')