Skip to content

Commit d6d17e5

Browse files
timtebeekTim te Beek
andauthored
Restore recipe hierarchy rows in SourcesFileResults (#6764)
* Restore recipe hierarchy rows in SourcesFileResults PR #6022 removed the recursive hierarchy recording in RecipeRunCycle to fix non-summable estimatedTimeSaving values. This had the side effect of breaking the Sankey visualization, which needs (parentRecipe, recipe) rows at every level to draw the full drill-down from top-level recipes through to leaf recipes that made changes. Restore the recursive recordSourceFileResult() method but pass 0 for estimatedTimeSaving on hierarchy rows, so that only leaf recipes carry effort values and the column remains summable. * Document the purpose of the test * Record why we show zero time savings --------- Co-authored-by: Tim te Beek <tim@mac.home>
1 parent e1183da commit d6d17e5

2 files changed

Lines changed: 72 additions & 0 deletions

File tree

rewrite-core/src/main/java/org/openrewrite/scheduling/RecipeRunCycle.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,33 @@ protected void recordSourceFileResultAndSearchResults(@Nullable SourceFile befor
261261
for (SearchResults.Row searchResult : searchMarkers) {
262262
searchResults.insertRow(ctx, searchResult);
263263
}
264+
265+
if (hierarchical) {
266+
recordSourceFileResult(beforePath, afterPath, recipeStack.subList(0, recipeStack.size() - 1), ctx);
267+
}
268+
}
269+
270+
private void recordSourceFileResult(@Nullable String beforePath, @Nullable String afterPath, List<Recipe> recipeStack, ExecutionContext ctx) {
271+
if (recipeStack.size() <= 1) {
272+
// No reason to record the synthetic root recipe which contains the recipe run
273+
return;
274+
}
275+
String parentName;
276+
if (recipeStack.size() == 2) {
277+
// Record the parent name as blank rather than CompositeRecipe when the parent is the synthetic root recipe
278+
parentName = "";
279+
} else {
280+
parentName = recipeStack.get(recipeStack.size() - 2).getName();
281+
}
282+
Recipe recipe = recipeStack.get(recipeStack.size() - 1);
283+
sourcesFileResults.insertRow(ctx, new SourcesFileResults.Row(
284+
beforePath,
285+
afterPath,
286+
parentName,
287+
recipe.getName(),
288+
0L, // Zero here, as we later sum only the recipes that themselves made changes
289+
cycle));
290+
recordSourceFileResult(beforePath, afterPath, recipeStack.subList(0, recipeStack.size() - 1), ctx);
264291
}
265292

266293
private @Nullable SourceFile handleError(Recipe recipe, SourceFile sourceFile, @Nullable SourceFile after,

rewrite-core/src/test/java/org/openrewrite/table/SourceFileResultsTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,49 @@ void hierarchical() {
5454
)
5555
);
5656
}
57+
58+
@Test
59+
void deepHierarchyRecordsAllLevels() {
60+
// Our Sankey visualization of recipe execution relies on the data table recording all levels of the hierarchy,
61+
// not just the leaf recipes. This test ensures that the data table includes rows for both the parent and leaf
62+
// recipes, even when the parent recipe doesn't directly perform any transformations (i.e., it has zero
63+
// estimated time saving).
64+
rewriteRun(
65+
spec -> spec
66+
.recipeFromYaml(
67+
//language=yml
68+
"""
69+
type: specs.openrewrite.org/v1beta/recipe
70+
name: test.GrandParent
71+
displayName: Grand parent recipe
72+
description: Three-level hierarchy.
73+
recipeList:
74+
- test.Parent
75+
---
76+
type: specs.openrewrite.org/v1beta/recipe
77+
name: test.Parent
78+
displayName: Parent recipe
79+
description: Middle level.
80+
recipeList:
81+
- org.openrewrite.text.ChangeText:
82+
toText: Hello!
83+
""",
84+
"test.GrandParent"
85+
).dataTable(SourcesFileResults.Row.class, rows -> {
86+
// Leaf recipe row + one hierarchy row for the parent recipe.
87+
// The root recipe (GrandParent) doesn't get its own row but appears
88+
// as the parent in production runs where a synthetic root wraps it.
89+
assertThat(rows).hasSize(2);
90+
assertThat(rows.get(0).getParentRecipe()).isEqualTo("test.Parent");
91+
assertThat(rows.get(0).getRecipe()).isEqualTo("org.openrewrite.text.ChangeText");
92+
// Hierarchy row with zero effort so the column remains summable
93+
assertThat(rows.get(1).getRecipe()).isEqualTo("test.Parent");
94+
assertThat(rows.get(1).getEstimatedTimeSaving()).isEqualTo(0L);
95+
}),
96+
text(
97+
"Hi",
98+
"Hello!"
99+
)
100+
);
101+
}
57102
}

0 commit comments

Comments
 (0)