Skip to content

Commit

Permalink
Fix googletest integration with CLion 2024.2 (#6741)
Browse files Browse the repository at this point in the history
* clion: fix UI call on non-UI thread

after 2024.2 CLion invokes createProcess on a background
thread, so UI access is no longer allowed in this context.
fix is backward compatible as we're allowed to use
invokeAndWait on EDT as well.

fixes #6740

* clion: add missing ReadActions to GoogleTest support

since the code was moved from the UI thread to the
background,the read action context is no longer available.
anyway in CLion 2024.3 this will be broken since ijplatform
will not be providing implicit read action context on EDT.
  • Loading branch information
ujohnny committed Sep 6, 2024
1 parent f12de9d commit c616753
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.common.collect.Iterables;
import com.google.idea.blaze.clwb.run.GoogleTestUtilAdapter;
import com.intellij.openapi.application.ReadAction;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Couple;
import com.intellij.psi.PsiElement;
Expand All @@ -38,7 +39,6 @@ public class CidrGoogleTestUtilAdapter implements GoogleTestUtilAdapter {
@Nullable
public static PsiElement findGoogleTestSymbol(
Project project, String suiteName, String testName) {
CidrGoogleTestFramework instance = CidrGoogleTestFramework.getInstance();
final String prefixedSuiteName = "suite:" + suiteName;
final String prefixedTestName = "test:" + testName;

Expand All @@ -47,15 +47,8 @@ public static PsiElement findGoogleTestSymbol(
(CidrTestScopeElement testElement) ->
prefixedSuiteName.equals(testElement.getSuiteName())
&& prefixedTestName.equals(testElement.getTestName()));
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
if (!processor.isFound()) {
return null;
}
CidrTestScopeElement testElement = processor.getFoundValue();
if (testElement == null) {
return null;
}
return testElement.getElement();

return getTestElement(project, processor);
}

@Nullable
Expand All @@ -82,16 +75,7 @@ public static PsiElement findGoogleTestInstantiationSymbol(
parameterizedSuiteName.equals(testElement.getSuiteName()));
}

CidrGoogleTestFramework instance = CidrGoogleTestFramework.getInstance();
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
if (!processor.isFound()) {
return null;
}
CidrTestScopeElement testElement = processor.getFoundValue();
if (testElement == null) {
return null;
}
return testElement.getElement();
return getTestElement(project, (FindFirstWithPredicateProcessor<CidrTestScopeElement>) processor);
}

/* TODO: Convert to not use CidrGoogleTestUtilObsolete. */
Expand Down Expand Up @@ -133,12 +117,15 @@ public PsiElement findGoogleTestSymbol(
CidrGoogleTestFramework instance = CidrGoogleTestFramework.getInstance();
FindFirstWithPredicateProcessor<CidrTestScopeElement> processor =
new FindFirstWithPredicateProcessor<>(predicate);
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
CidrTestScopeElement testScopeElement = processor.getFoundValue();
if (testScopeElement == null) {
return null;
}
return testScopeElement.getElement();

return ReadAction.compute(() -> {
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
CidrTestScopeElement testScopeElement = processor.getFoundValue();
if (testScopeElement == null) {
return null;
}
return testScopeElement.getElement();
});
}

private static Collection<CidrTestScopeElement> findGoogleTestSymbols(
Expand All @@ -151,8 +138,11 @@ protected boolean accept(CidrTestScopeElement cidrTestScopeElement) {
return predicate.test(cidrTestScopeElement);
}
};
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
return processor.getResults();

return ReadAction.compute(() -> {
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
return processor.getResults();
});
}

private static class FindFirstWithPredicateProcessor<T> extends FindProcessor<T> {
Expand All @@ -167,4 +157,19 @@ protected boolean accept(T t) {
return !isFound() && predicate.test(t);
}
}

private static PsiElement getTestElement(Project project, FindFirstWithPredicateProcessor<CidrTestScopeElement> processor) {
return ReadAction.compute(() -> {
CidrGoogleTestFramework instance = CidrGoogleTestFramework.getInstance();
instance.consumeTestObjects(project, GlobalSearchScope.allScope(project), processor);
if (!processor.isFound()) {
return null;
}
CidrTestScopeElement testElement = processor.getFoundValue();
if (testElement == null) {
return null;
}
return testElement.getElement();
});
}
}
43 changes: 23 additions & 20 deletions clwb/src/com/google/idea/blaze/clwb/run/BlazeCidrLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import com.intellij.execution.runners.ExecutionEnvironment;
import com.intellij.execution.ui.ConsoleView;
import com.intellij.ide.util.PropertiesComponent;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.ui.Messages;
import com.intellij.xdebugger.XDebugSession;
Expand Down Expand Up @@ -135,26 +136,28 @@ private ProcessHandler createProcess(CommandLineState state, List<String> extraB
Preconditions.checkNotNull(ProjectViewManager.getInstance(project).getProjectViewSet());

if (shouldDisplayBazelTestFilterWarning()) {
String messageContents =
"<html>The Google Test framework did not apply test filtering correctly before "
+ "git commit <a href='https://github.com/google/googletest/commit/"
+ "ba96d0b1161f540656efdaed035b3c062b60e006"
+ "'>ba96d0b<a>.<br/>"
+ "Please ensure you are past this commit if you are using it.<br/><br/>"
+ "More information on the bazel <a href='https://github.com/bazelbuild/bazel/issues/"
+ "4411'>issue</a></html>";

int selectedOption =
Messages.showDialog(
getProject(),
messageContents,
"Please update 'Google Test' past ba96d0b...",
new String[] {"Close", "Don't show again"},
0, // Default to "Close"
Messages.getWarningIcon());
if (selectedOption == 1) {
PropertiesComponent.getInstance().setValue(DISABLE_BAZEL_GOOGLETEST_FILTER_WARNING, "true");
}
ApplicationManager.getApplication().invokeAndWait(() -> {
String messageContents =
"<html>The Google Test framework did not apply test filtering correctly before "
+ "git commit <a href='https://github.com/google/googletest/commit/"
+ "ba96d0b1161f540656efdaed035b3c062b60e006"
+ "'>ba96d0b<a>.<br/>"
+ "Please ensure you are past this commit if you are using it.<br/><br/>"
+ "More information on the bazel <a href='https://github.com/bazelbuild/bazel/issues/"
+ "4411'>issue</a></html>";

int selectedOption =
Messages.showDialog(
getProject(),
messageContents,
"Please update 'Google Test' past ba96d0b...",
new String[] {"Close", "Don't show again"},
0, // Default to "Close"
Messages.getWarningIcon());
if (selectedOption == 1) {
PropertiesComponent.getInstance().setValue(DISABLE_BAZEL_GOOGLETEST_FILTER_WARNING, "true");
}
});
}

BlazeCommand.Builder commandBuilder =
Expand Down

0 comments on commit c616753

Please sign in to comment.