JavaScript: in tests the before_recipe hook to run after idempotence check (#6738)

* beforeSpec hook after idempotency check

* Move beforeRecipe hook aafter idempotence check

Author: greg-at-moderne

rewrite-javascript/rewrite/src/test/rewrite-test.ts

@@ -100,6 +100,25 @@ export class RecipeSpec {
             allParsed.push(...parsed);
         }
 
+        // Apply beforeRecipe hooks (after idempotence check, before recipe execution)
+        for (let i = 0; i < allParsed.length; i++) {
+            const [spec, sourceFile] = allParsed[i];
+            if (spec.beforeRecipe) {
+                const b = spec.beforeRecipe(sourceFile);
+                if (b !== undefined) {
+                    if (b instanceof Promise) {
+                        const mapped = await b;
+                        if (mapped === undefined) {
+                            throw new Error("Expected beforeRecipe to return a SourceFile, but got undefined. Did you forget a return statement?");
+                        }
+                        allParsed[i] = [spec, mapped];
+                    } else {
+                        allParsed[i] = [spec, b as SourceFile];
+                    }
+                }
+            }
+        }
+
         const changeset = (await scheduleRun(this.recipe,
             allParsed.map(([_, sourceFile]) => sourceFile),
             this.recipeExecutionContext)).changeset;
@@ -131,17 +150,18 @@ export class RecipeSpec {
     private async expectResultsToMatchAfter(specs: SourceSpec<any>[], changeset: Result[], parsed: [SourceSpec<any>, SourceFile][]) {
         for (const spec of specs) {
             const matchingSpec = parsed.find(([s, _]) => s === spec);
-            const after = changeset.find(c => {
+            const result = changeset.find(c => {
                 if (c.before) {
                     return c.before === matchingSpec![1];
                 } else if (c.after) {
                     const matchingSpec = specs.find(s => s.path === c.after!.sourcePath);
                     return !!matchingSpec;
                 }
-            })?.after;
+            });
+            const after = result?.after;
 
             if (!spec.after) {
-                if (after) {
+                if (after && after !== result?.before) {
                     expect(await TreePrinters.print(after)).toEqual(dedent(spec.before!));
                     // TODO: Consider throwing an error, as there should typically have been no change to the LST
                     // fail("Expected after to be undefined.");
@@ -200,21 +220,7 @@ export class RecipeSpec {
         for await (const sourceFile of parser.parse(...before.map(([_, parserInput]) => parserInput))) {
             parsed.push(sourceFile);
         }
-        const specToParsed: [SourceSpec<any>, SourceFile][] = before.map(([spec, _], i) => [spec, parsed[i]]);
-        return await mapAsync(specToParsed, async ([spec, sourceFile]) => {
-            const b = spec.beforeRecipe ? spec.beforeRecipe(sourceFile) : sourceFile;
-            if (b !== undefined) {
-                if (b instanceof Promise) {
-                    const mapped = await b;
-                    if (mapped === undefined) {
-                        throw new Error("Expected beforeRecipe to return a SourceFile, but got undefined. Did you forget a return statement?");
-                    }
-                    return [spec, mapped];
-                }
-                return [spec, b as SourceFile];
-            }
-            return [spec, sourceFile];
-        });
+        return before.map(([spec, _], i): [SourceSpec<any>, SourceFile] => [spec, parsed[i]]);
     }
 
     private async expectWhitespaceNotToContainNonwhitespaceCharacters(parsed: [SourceSpec<any>, SourceFile][]) {

rewrite-javascript/rewrite/test/test/rewrite-test.test.ts

@@ -4,6 +4,8 @@ import {text} from "../../src/text";
 import {json} from "../../src/json";
 import {ChangeText} from "../../fixtures/change-text";
 import {ExecutionContext, ScanningRecipe} from "../../src";
+import {foundSearchResult, MarkersKind} from "../../src/markers";
+import {PlainText} from "../../src/text";
 
 describe("rewrite test", () => {
     const spec = new RecipeSpec();
@@ -40,6 +42,30 @@ describe("rewrite test", () => {
         }
     ));
 
+    test("beforeRecipe can modify AST without breaking idempotence check", async () => {
+        // given
+        const sut = new RecipeSpec();
+        const markerFound: boolean[] = [];
+
+        // when
+        await sut.rewriteRun(
+            {
+                ...text("hello"),
+                beforeRecipe: (sourceFile: PlainText) => {
+                    return foundSearchResult(sourceFile, "test");
+                },
+                afterRecipe: (sourceFile: PlainText) => {
+                    const marker = sourceFile.markers.markers
+                        .find(m => m.kind === MarkersKind.SearchResult);
+                    markerFound.push(marker !== undefined);
+                }
+            }
+        );
+
+        // then
+        expect(markerFound).toEqual([true]);
+    });
+
     test("two kinds of sources and a scanning recipe", async () => {
         // given
         const sut = new RecipeSpec();