Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actually support Recipe in addApplicableTest & addSingleSourceApplicableTest #2861

Merged
merged 1 commit into from
Feb 21, 2023

Conversation

JLLeitschuh
Copy link
Collaborator

@JLLeitschuh JLLeitschuh commented Feb 17, 2023

Recipes used in applicability tests will have their own applicability tests invoked now.

This means that expensive recipes with light singleSourceApplicableTest can be used without incurring a performance overhead of invoking the recipe visitor every time.

Comment on lines -139 to -144
static List<Result> createAndProcessResults(
List<? extends SourceFile> before,
List<? extends SourceFile> after,
ExecutionContext ctx,
Map<UUID, Stack<Recipe>> recipeThatAddedOrDeletedSourceFile
) {
Copy link
Collaborator Author

@JLLeitschuh JLLeitschuh Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into RecipeSchedulerUtil but otherwise unchanged IIRC

@JLLeitschuh JLLeitschuh force-pushed the feat/JLL/full-visitors-as-applicability-tests branch 4 times, most recently from fe993b5 to 6a5c70d Compare February 21, 2023 17:46
Comment on lines +562 to +570
for (S newS : next) {
newS.getMarkers().findFirst(Markup.Error.class).ifPresent(m -> {
if (m.getException() instanceof RecipeRunException) {
throw (RecipeRunException) m.getException();
} else {
throw new RuntimeException("Applicable Test Failed", m.getException());
}
});
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkschneider is there a better/more appropriate way to do this?

@JLLeitschuh JLLeitschuh force-pushed the feat/JLL/full-visitors-as-applicability-tests branch from 6a5c70d to efdd3db Compare February 21, 2023 18:17
…plicableTest`

Recipes used in applicability tests will have their own applicabibility tests invoked now.

This means that expensive recipes with light `singleSourceApplicableTest` can be used
without incurring a performance overhead of invoking the recipe visitor every time.
@JLLeitschuh JLLeitschuh force-pushed the feat/JLL/full-visitors-as-applicability-tests branch from efdd3db to 9983adc Compare February 21, 2023 18:50
@sambsnyd sambsnyd merged commit 9b72d9a into main Feb 21, 2023
@sambsnyd sambsnyd deleted the feat/JLL/full-visitors-as-applicability-tests branch February 21, 2023 19:24
@JLLeitschuh JLLeitschuh self-assigned this Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants