How to resolve expected recipe cycle mismatch

How to resolve expected recipe cycle mismatch

Are you seeing an error message like this:

Expected recipe to complete in 1 cycle, but took 2 cycles. This usually indicates the recipe is making changes after it should have stabilized.

or

Expected recipe to complete in 1 cycle, but took at least one more cycle. Between the last two executed cycles there were changes to "TestClass.java"

Essentially these errors are telling you the number of cycles your recipe made changes to the AST in did not match the expected number.

What is a cycle? A cycle refers to one full run through of your recipe where it visits all nodes on the AST for a given file. By default the OpenRewrite test suite will expect your recipe to only require one cycle of changes in the input AST to match the expected output AST. The test suite does this by running your recipe through two cycles and asserting no changes are made on the second cycle.

For the default recipe test configuration if your recipe continues to make changes on the second cycle this is where we will see errors like the ones mentioned above.

Resolving expected cycles mismatch in OpenRewrite tests

Changing the expected number of cycles

Perhaps it is by design your recipe will require more than one cycle to complete the necessary changes? In this situation we can change the default number of expected cycles. By default, the OpenRewrite recipe tests will run two cycles and expect the recipe to complete all changes in one fewer cycles than the tests run (in this case is is the two expected minus one to get one expected cycle).

rewriteRun(
        // Set the number of cycles the test should run and how many cycles we expect changes to occur in
        // By default these are 2 and 1 respectively 
        spec -> spec.cycles(4)
            .expectedCyclesThatMakeChanges(2),
        java(
                """
                <INPUT_JAVA>
                        """,
                        """
                <OUTPUT_JAVA>
                        """
    )
);

Wherever practical it is best to keep your recipes single cycle. Overall single cycle recipes are more efficient as the AST will only need to be parsed once. See stay single cycle.

Making your recipe single cycle

If your recipe is taking more than one cycle and this is not expected often the debugger is the best way to determine where the unexpected behaviour is coming from. Use the debugger to step over your recipe and print out the AST to see which cycle is still causing changes erroneously.

The most common issue is the recipe is unconditionally applying your change to the AST. The way to resolve this is to check the AST to see if it needs updating. A good way to do this is to add guards to your recipe visit methods that check if the change is required. For example if you are adding an annotation to every field in each class you might have a simple visitor like this:

public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
    multiVariable = super.visitVariableDeclarations(multiVariable, executionContext);

    return addDummyBetaAnnotation(multiVariable);
}

private J.VariableDeclarations addDummyBetaAnnotation(J.VariableDeclarations multiVariable) {
    maybeAddImport(Beta.class.getTypeName());

    return JavaTemplate.builder("@Beta")
                    .javaParser(JavaParser.fromJavaVersion().classpath("guava"))
                    .imports(Beta.class.getTypeName())
                    .build()
            .apply(getCursor(), multiVariable.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::toString)));
}

When you apply this recipe over multiple cycles it will not work. You will get an error message like this: org.opentest4j.AssertionFailedError: [Expected recipe to complete in 1 cycle, but took at least one more cycle. Between the last two executed cycles there were changes to "TestClass.java"]

Each cycle the visitVariableDeclarations method is called the visitor will unconditionally add the annotation even if it already exists on the field. To make visitVariableDeclarations more robust we need to add a guard like so:

public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
    multiVariable = super.visitVariableDeclarations(multiVariable, executionContext);
    // Don't add the annotation if there are any annotations 
    if (multiVariable.getLeadingAnnotations().size() >= 1) {
        return multiVariable;
    }

    return addDummyBetaAnnotation(multiVariable);
}

Adding this guard resolves the cycle error however it is not robust. If there is another annotation (e.g. @Deprecated) on the field we will not add our @Beta annotation. To resolve this we can use an annotation matcher to see if any of the existing annotations match.

private static final AnnotationMatcher BETA_ANNOTATION_MATCHER = new AnnotationMatcher(Beta.class.getTypeName());

public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
    multiVariable = super.visitVariableDeclarations(multiVariable, executionContext);
    // Don't add @Beta if @Beta annotation already exists
    if (shouldAddAnnotation(multiVariable)) {
        return multiVariable;
    }

    return addDummyBetaAnnotation(multiVariable);
}

private boolean shouldAddAnnotation(J.VariableDeclarations multiVariable) {
    return multiVariable.getLeadingAnnotations().stream()
            .anyMatch(BETA_ANNOTATION_MATCHER::matches);
}

Summary

  • By default the OpenRewrite test suite will:

    • Run a recipe through two cycles

    • Expect only one cycle to make changes to the AST

  • In tests the number of cycles run and how many are expected to make changes are configurable through the RecipeSpec

  • Often adding guards to visit methods are the best ways to prevent unesscary AST modifications

References

Support our work