From 69b2163ce59d5a8d12c44dc29f92ddb8aa7e7e82 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Thu, 30 Jun 2022 17:31:06 +0200 Subject: [PATCH] optimize exception handling in logger GitOrigin-RevId: dc29f629b89fcb3a28ec5d36279f3d8e111e1795 --- ...rnalSystemRunConfigurationJavaExtensionTest.kt | 2 +- .../ExternalSystemStorageTest.kt | 4 ++-- .../application/PooledCoroutineContextTest.kt | 2 +- .../ide/actions/BadActionShortcutCheckTest.java | 2 +- .../impl/NonBlockingReadActionTest.java | 5 ++--- .../testSrc/com/intellij/openapi/progress/util.kt | 2 +- .../vfs/newvfs/persistent/PersistentFsTest.java | 2 +- .../org/jetbrains/concurrency/AsyncPromiseTest.kt | 2 +- .../testFramework/LoggedErrorProcessor.java | 15 ++++++++++----- .../StringTemplateExpressionManipulatorTest.kt | 2 +- .../output/MavenBuildToolLogTestUtils.java | 2 +- .../importing/InvalidEnvironmentImportingTest.kt | 2 +- .../maven/testFramework/MavenTestCase.java | 4 ++-- 13 files changed, 25 insertions(+), 21 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/java/execution/ExternalSystemRunConfigurationJavaExtensionTest.kt b/java/java-tests/testSrc/com/intellij/java/execution/ExternalSystemRunConfigurationJavaExtensionTest.kt index 74cddbc2d3c3..4d0d56abef30 100644 --- a/java/java-tests/testSrc/com/intellij/java/execution/ExternalSystemRunConfigurationJavaExtensionTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/execution/ExternalSystemRunConfigurationJavaExtensionTest.kt @@ -35,7 +35,7 @@ class ExternalSystemRunConfigurationJavaExtensionTest : RunConfigurationJavaExte LoggedErrorProcessor.executeWith(object : LoggedErrorProcessor() { override fun processError(category: String, message: String, details: Array, t: Throwable?): Set = // don't fail this if `LOG.error()` was called for our exception somewhere - if (t is FakeExecutionException) EnumSet.noneOf(Action::class.java) else EnumSet.allOf(Action::class.java) + if (t is FakeExecutionException) Action.NONE else Action.ALL }) { runInEdtAndWait { ExecutionEnvironmentBuilder.create(DefaultRunExecutor.getRunExecutorInstance(), configuration).buildAndExecute() diff --git a/platform/external-system-impl/testSrc/com/intellij/openapi/externalSystem/configurationStore/ExternalSystemStorageTest.kt b/platform/external-system-impl/testSrc/com/intellij/openapi/externalSystem/configurationStore/ExternalSystemStorageTest.kt index 1bb40a792ffe..4f14181802ae 100644 --- a/platform/external-system-impl/testSrc/com/intellij/openapi/externalSystem/configurationStore/ExternalSystemStorageTest.kt +++ b/platform/external-system-impl/testSrc/com/intellij/openapi/externalSystem/configurationStore/ExternalSystemStorageTest.kt @@ -872,8 +872,8 @@ class ExternalSystemStorageTest { private fun suppressLogs(action: () -> Unit) { LoggedErrorProcessor.executeWith(object : LoggedErrorProcessor() { override fun processError(category: String, message: String, details: Array, t: Throwable?): Set = - if (message.contains("Trying to load multiple modules with the same name.")) EnumSet.noneOf(Action::class.java) - else EnumSet.allOf(Action::class.java) + if (message.contains("Trying to load multiple modules with the same name.")) Action.NONE + else Action.ALL }) { action() } diff --git a/platform/platform-tests/testSrc/com/intellij/application/PooledCoroutineContextTest.kt b/platform/platform-tests/testSrc/com/intellij/application/PooledCoroutineContextTest.kt index 6391979b0d70..d586bab823fa 100644 --- a/platform/platform-tests/testSrc/com/intellij/application/PooledCoroutineContextTest.kt +++ b/platform/platform-tests/testSrc/com/intellij/application/PooledCoroutineContextTest.kt @@ -47,7 +47,7 @@ class PooledCoroutineContextTest : UsefulTestCase() { LoggedErrorProcessor.executeWith(object : LoggedErrorProcessor() { override fun processError(category: String, message: String, details: Array, t: Throwable?): MutableSet { throwable = t - return EnumSet.noneOf(Action::class.java) + return Action.NONE } }, block) return throwable diff --git a/platform/platform-tests/testSrc/com/intellij/ide/actions/BadActionShortcutCheckTest.java b/platform/platform-tests/testSrc/com/intellij/ide/actions/BadActionShortcutCheckTest.java index 89b38e58c07b..7adda3dd4093 100644 --- a/platform/platform-tests/testSrc/com/intellij/ide/actions/BadActionShortcutCheckTest.java +++ b/platform/platform-tests/testSrc/com/intellij/ide/actions/BadActionShortcutCheckTest.java @@ -21,7 +21,7 @@ public class BadActionShortcutCheckTest extends LightPlatformTestCase { protected void runTestRunnable(@NotNull ThrowableRunnable testRunnable) throws Throwable { LoggedErrorProcessor.executeWith(new LoggedErrorProcessor() { @Override - public boolean processWarn(@NotNull String category, String message, Throwable t) { + public boolean processWarn(@NotNull String category, @NotNull String message, Throwable t) { myLoggedWarnings.add(message); return super.processWarn(category, message, t); } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java index 30d8ff49dce3..366e37d2fec6 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java @@ -38,7 +38,6 @@ import org.jetbrains.concurrency.Promise; import java.io.IOException; import java.util.ArrayList; -import java.util.EnumSet; import java.util.List; import java.util.Set; import java.util.concurrent.*; @@ -456,10 +455,10 @@ public class NonBlockingReadActionTest extends LightPlatformTestCase { AtomicReference loggedError = new AtomicReference<>(); LoggedErrorProcessor.executeWith(new LoggedErrorProcessor() { @Override - public Set processError(@NotNull String category, @NotNull String message, String @NotNull [] details, @Nullable Throwable t) { + public @NotNull Set processError(@NotNull String category, @NotNull String message, String @NotNull [] details, @Nullable Throwable t) { assertNotNull(t); loggedError.set(t); - return EnumSet.noneOf(Action.class); + return Action.NONE; } }, ()->runnable.accept(loggedError)); } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/progress/util.kt b/platform/platform-tests/testSrc/com/intellij/openapi/progress/util.kt index 97d1b40d8dac..b3a6712f20fa 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/progress/util.kt +++ b/platform/platform-tests/testSrc/com/intellij/openapi/progress/util.kt @@ -123,7 +123,7 @@ fun loggedError(canThrow: Semaphore): Throwable { override fun processError(category: String, message: String, details: Array, t: Throwable?): Set { throwable = t!! gotIt.up() - return EnumSet.noneOf(Action::class.java) + return Action.NONE } }) { canThrow.up() diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java index 02cca13d4052..bf5e05bfc9cd 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java @@ -206,7 +206,7 @@ public class PersistentFsTest extends BareTestFixtureTestCase { int[] logCount = {0}; LoggedErrorProcessor.executeWith(new LoggedErrorProcessor() { @Override - public boolean processWarn(@NotNull String category, String message, Throwable t) { + public boolean processWarn(@NotNull String category, @NotNull String message, Throwable t) { if (message.contains(jarFile.getName())) logCount[0]++; return super.processWarn(category, message, t); } diff --git a/platform/platform-tests/testSrc/org/jetbrains/concurrency/AsyncPromiseTest.kt b/platform/platform-tests/testSrc/org/jetbrains/concurrency/AsyncPromiseTest.kt index 522e60d361d8..a8b2c82b8cb6 100644 --- a/platform/platform-tests/testSrc/org/jetbrains/concurrency/AsyncPromiseTest.kt +++ b/platform/platform-tests/testSrc/org/jetbrains/concurrency/AsyncPromiseTest.kt @@ -359,7 +359,7 @@ class AsyncPromiseTest { LoggedErrorProcessor.executeWith(object : LoggedErrorProcessor() { override fun processError(category: String, message: String, details: Array, t: Throwable?): Set { loggedError.set(true) - return EnumSet.noneOf(Action::class.java) + return Action.NONE } }) { val promise = ReadAction.nonBlocking { diff --git a/platform/testFramework/core/src/com/intellij/testFramework/LoggedErrorProcessor.java b/platform/testFramework/core/src/com/intellij/testFramework/LoggedErrorProcessor.java index 3f6b82017242..e460382f9da0 100644 --- a/platform/testFramework/core/src/com/intellij/testFramework/LoggedErrorProcessor.java +++ b/platform/testFramework/core/src/com/intellij/testFramework/LoggedErrorProcessor.java @@ -39,7 +39,7 @@ public class LoggedErrorProcessor { AtomicReference error = new AtomicReference<>(); executeWith(new LoggedErrorProcessor() { @Override - public boolean processError(@NotNull String category, String message, Throwable t, String @NotNull [] details) { + public boolean processError(@NotNull String category, @NotNull String message, Throwable t, String @NotNull [] details) { Assert.assertNotNull("Unexpected error without Throwable: " + message, t); if (!error.compareAndSet(null, t)) { Assert.fail("Multiple errors were reported: " + error.get().getMessage() + " and " + t.getMessage()); @@ -58,23 +58,28 @@ public class LoggedErrorProcessor { * * @see TestLoggerFactory.TestLogger#warn(String, Throwable) */ - public boolean processWarn(@NotNull String category, String message, Throwable t) { + public boolean processWarn(@NotNull String category, @NotNull String message, @Nullable Throwable t) { return true; } - public enum Action {LOG, STDERR, RETHROW} + public enum Action { + LOG, STDERR, RETHROW; + public static final EnumSet ALL = EnumSet.allOf(Action.class); + public static final EnumSet NONE = EnumSet.noneOf(Action.class); + } /** * Returns a set of actions to be performed by {@link TestLoggerFactory.TestLogger#error(String, Throwable, String...)} on the given log event. */ + @NotNull public Set processError(@NotNull String category, @NotNull String message, String @NotNull [] details, @Nullable Throwable t) { var process = processError(category, message, t, details); - return process ? EnumSet.allOf(Action.class) : EnumSet.noneOf(Action.class); + return process ? Action.ALL : Action.NONE; } /** @deprecated use/override {@link #processError(String, String, String[], Throwable)} instead */ @Deprecated(forRemoval = true) - public boolean processError(@NotNull String category, String message, Throwable t, String @NotNull [] details) { + public boolean processError(@NotNull String category, @NotNull String message, @Nullable Throwable t, String @NotNull [] details) { return true; } } diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/psi/StringTemplateExpressionManipulatorTest.kt b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/psi/StringTemplateExpressionManipulatorTest.kt index 8ef553e4310b..727228fa8764 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/psi/StringTemplateExpressionManipulatorTest.kt +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/psi/StringTemplateExpressionManipulatorTest.kt @@ -88,7 +88,7 @@ class StringTemplateExpressionManipulatorTest : KotlinLightCodeInsightFixtureTes private fun suppressFallingOnLogError(call: () -> Unit) { LoggedErrorProcessor.executeWith(object : LoggedErrorProcessor() { override fun processError(category: String, message: String, details: Array, t: Throwable?): Set = - EnumSet.noneOf(Action::class.java) + Action.NONE }) { call() } diff --git a/plugins/maven/src/test/java/org/jetbrains/idea/maven/externalSystemIntegration/output/MavenBuildToolLogTestUtils.java b/plugins/maven/src/test/java/org/jetbrains/idea/maven/externalSystemIntegration/output/MavenBuildToolLogTestUtils.java index 1a8008926dd7..aed7f0462af0 100644 --- a/plugins/maven/src/test/java/org/jetbrains/idea/maven/externalSystemIntegration/output/MavenBuildToolLogTestUtils.java +++ b/plugins/maven/src/test/java/org/jetbrains/idea/maven/externalSystemIntegration/output/MavenBuildToolLogTestUtils.java @@ -45,7 +45,7 @@ public abstract class MavenBuildToolLogTestUtils extends LightIdeaTestCase { public static void failOnWarns(ThrowableRunnable runnable) throws Throwable { LoggedErrorProcessor.executeWith(new LoggedErrorProcessor() { @Override - public boolean processWarn(@NotNull String category, String message, Throwable t) { + public boolean processWarn(@NotNull String category, @NotNull String message, Throwable t) { fail(message + t); return false; } diff --git a/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/InvalidEnvironmentImportingTest.kt b/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/InvalidEnvironmentImportingTest.kt index b41cfe7c3889..6da58a5a627f 100644 --- a/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/InvalidEnvironmentImportingTest.kt +++ b/plugins/maven/src/test/java/org/jetbrains/idea/maven/importing/InvalidEnvironmentImportingTest.kt @@ -85,7 +85,7 @@ class InvalidEnvironmentImportingTest : MavenMultiVersionImportingTestCase() { private fun loggedErrorProcessor(search: String) = object : LoggedErrorProcessor() { override fun processError(category: String, message: String, details: Array, t: Throwable?): Set = - if (message.contains(search)) EnumSet.noneOf(Action::class.java) else EnumSet.allOf(Action::class.java) + if (message.contains(search)) Action.NONE else Action.ALL } private fun assertEvent(description: String = "Asserted", predicate: (BuildEvent) -> Boolean) { diff --git a/plugins/maven/testFramework/src/com/intellij/maven/testFramework/MavenTestCase.java b/plugins/maven/testFramework/src/com/intellij/maven/testFramework/MavenTestCase.java index 07168b893152..e59d6a54dd09 100644 --- a/plugins/maven/testFramework/src/com/intellij/maven/testFramework/MavenTestCase.java +++ b/plugins/maven/testFramework/src/com/intellij/maven/testFramework/MavenTestCase.java @@ -153,11 +153,11 @@ public abstract class MavenTestCase extends UsefulTestCase { protected void runBare(@NotNull ThrowableRunnable testRunnable) throws Throwable { LoggedErrorProcessor.executeWith(new LoggedErrorProcessor() { @Override - public Set processError(@NotNull String category, @NotNull String message, String @NotNull [] details, @Nullable Throwable t) { + public @NotNull Set processError(@NotNull String category, @NotNull String message, String @NotNull [] details, @Nullable Throwable t) { boolean intercept = t != null && ( StringUtil.notNullize(t.getMessage()).contains("The network name cannot be found") && message.contains("Couldn't read shelf information") || "JDK annotations not found".equals(t.getMessage()) && "#com.intellij.openapi.projectRoots.impl.JavaSdkImpl".equals(category)); - return intercept ? EnumSet.noneOf(Action.class) : EnumSet.allOf(Action.class); + return intercept ? Action.NONE : Action.ALL; } }, () -> super.runBare(testRunnable)); }