From d3f830d4ef34de09e221c3c8508b96d19372e270 Mon Sep 17 00:00:00 2001 From: Christian Sagel Date: Wed, 18 Dec 2024 13:58:19 +0100 Subject: [PATCH] Add a virtual method for customizing the exception message for unity task (#200) ## Description Add a method that can be overridden in the `UnityTask` for customizing the exception message. ## Changes * ![ADD] `UnityTask.composeExceptionMessage,` virtual method for customizing the exception message when the task fails. [NEW]: https://resources.atlas.wooga.com/icons/icon_new.svg "New" [ADD]: https://resources.atlas.wooga.com/icons/icon_add.svg "Add" [IMPROVE]: https://resources.atlas.wooga.com/icons/icon_improve.svg "Improve" [CHANGE]: https://resources.atlas.wooga.com/icons/icon_change.svg "Change" [FIX]: https://resources.atlas.wooga.com/icons/icon_fix.svg "Fix" [UPDATE]: https://resources.atlas.wooga.com/icons/icon_update.svg "Update" [BREAK]: https://resources.atlas.wooga.com/icons/icon_break.svg "Remove" [REMOVE]: https://resources.atlas.wooga.com/icons/icon_remove.svg "Remove" [IOS]: https://resources.atlas.wooga.com/icons/icon_iOS.svg "iOS" [ANDROID]: https://resources.atlas.wooga.com/icons/icon_android.svg "Android" [WEBGL]: https://resources.atlas.wooga.com/icons/icon_webGL.svg "WebGL" [GRADLE]: https://resources.atlas.wooga.com/icons/icon_gradle.svg "GRADLE" [UNITY]: https://resources.atlas.wooga.com/icons/icon_unity.svg "Unity" [LINUX]: https://resources.atlas.wooga.com/icons/icon_linux.svg "Linux" [WIN]: https://resources.atlas.wooga.com/icons/icon_windows.svg "Windows" [MACOS]: https://resources.atlas.wooga.com/icons/icon_iOS.svg "macOS" --- .../DefaultUnityPluginTestOptions.groovy | 6 ++ .../unity/UnityPluginTestOptions.groovy | 2 + .../gradle/unity/UnityIntegrationSpec.groovy | 28 +++++++-- .../unity/UnityTaskIntegrationSpec.groovy | 5 +- .../tasks/FailTaskIntegrationSpec.groovy | 62 +++++++++++++++++++ .../unity/testutils/GradleRunResult.groovy | 9 +-- .../wooga/gradle/unity/UnityTask.groovy | 22 ++++--- 7 files changed, 116 insertions(+), 18 deletions(-) create mode 100644 src/integrationTest/groovy/wooga/gradle/unity/tasks/FailTaskIntegrationSpec.groovy diff --git a/src/integrationTest/groovy/com/wooga/spock/extensions/unity/DefaultUnityPluginTestOptions.groovy b/src/integrationTest/groovy/com/wooga/spock/extensions/unity/DefaultUnityPluginTestOptions.groovy index c6f9f58e..2e3b8679 100644 --- a/src/integrationTest/groovy/com/wooga/spock/extensions/unity/DefaultUnityPluginTestOptions.groovy +++ b/src/integrationTest/groovy/com/wooga/spock/extensions/unity/DefaultUnityPluginTestOptions.groovy @@ -27,6 +27,7 @@ class DefaultUnityPluginTestOptions implements UnityPluginTestOptions { Boolean addMockTask = true Boolean forceMockTaskRun = true Boolean clearMockTaskActions = false + Boolean writeMockExecutable = true boolean applyPlugin() { applyPlugin @@ -36,6 +37,11 @@ class DefaultUnityPluginTestOptions implements UnityPluginTestOptions { unityPath } + @Override + boolean writeMockExecutable() { + writeMockExecutable + } + boolean addPluginTestDefaults() { addPluginTestDefaults } diff --git a/src/integrationTest/groovy/com/wooga/spock/extensions/unity/UnityPluginTestOptions.groovy b/src/integrationTest/groovy/com/wooga/spock/extensions/unity/UnityPluginTestOptions.groovy index 473cb2c5..9225affb 100644 --- a/src/integrationTest/groovy/com/wooga/spock/extensions/unity/UnityPluginTestOptions.groovy +++ b/src/integrationTest/groovy/com/wooga/spock/extensions/unity/UnityPluginTestOptions.groovy @@ -41,6 +41,8 @@ import java.lang.annotation.Target UnityPathResolution unityPath() default UnityPathResolution.Mock + boolean writeMockExecutable() default true + boolean addPluginTestDefaults() default true boolean disableAutoActivateAndLicense() default true diff --git a/src/integrationTest/groovy/wooga/gradle/unity/UnityIntegrationSpec.groovy b/src/integrationTest/groovy/wooga/gradle/unity/UnityIntegrationSpec.groovy index e793164a..24d4d84f 100644 --- a/src/integrationTest/groovy/wooga/gradle/unity/UnityIntegrationSpec.groovy +++ b/src/integrationTest/groovy/wooga/gradle/unity/UnityIntegrationSpec.groovy @@ -19,10 +19,12 @@ package wooga.gradle.unity import com.wooga.gradle.PlatformUtils import com.wooga.gradle.test.IntegrationSpec -import com.wooga.gradle.test.executable.FakeExecutables +import com.wooga.gradle.test.mock.MockExecutable import com.wooga.spock.extensions.unity.DefaultUnityPluginTestOptions import com.wooga.spock.extensions.unity.UnityPathResolution import com.wooga.spock.extensions.unity.UnityPluginTestOptions +import groovy.transform.stc.ClosureParams +import groovy.transform.stc.FromString import wooga.gradle.unity.tasks.Unity import wooga.gradle.unity.utils.ProjectSettingsFile @@ -100,8 +102,9 @@ abstract class UnityIntegrationSpec extends IntegrationSpec { switch (options.unityPath()) { case UnityPathResolution.Mock: - mockUnityFile = createMockUnity() - addUnityPathToExtension(mockUnityFile.path) + if (options.writeMockExecutable()) { + writeMockExecutable() + } break case UnityPathResolution.Default: @@ -132,6 +135,24 @@ abstract class UnityIntegrationSpec extends IntegrationSpec { projectSettingsFile } + /** + * Writes the mock executable with a predetermined location + */ + protected void writeMockExecutable(@ClosureParams(value = FromString, options = "com.wooga.gradle.test.mock.MockExecutable") + Closure configure = null) { + // Create and configure the file to be written + def mockUnity = new MockExecutable("fakeUnity.bat") + mockUnity.withText(mockUnityStartupMessage) + if (configure != null) { + configure(mockUnity) + } + // Write the file + mockUnityFile = mockUnity.toDirectory(unityMainDirectory) + // Write its location onto the unity extension + addUnityPathToExtension(mockUnityFile.path) + } + + // TODO: Refactor away protected File createMockUnity(String extraLog = null, int exitValue=0) { def mockUnityFile = createFile("fakeUnity.bat", unityMainDirectory).with { delete() @@ -165,7 +186,6 @@ abstract class UnityIntegrationSpec extends IntegrationSpec { """.readLines().collect{it.stripIndent().trim() }.findAll {!it.empty}.join("\n") } return mockUnityFile - } void setLicenseDirectory() { diff --git a/src/integrationTest/groovy/wooga/gradle/unity/UnityTaskIntegrationSpec.groovy b/src/integrationTest/groovy/wooga/gradle/unity/UnityTaskIntegrationSpec.groovy index 95980e2a..f2ce6cb4 100644 --- a/src/integrationTest/groovy/wooga/gradle/unity/UnityTaskIntegrationSpec.groovy +++ b/src/integrationTest/groovy/wooga/gradle/unity/UnityTaskIntegrationSpec.groovy @@ -21,14 +21,11 @@ import com.wooga.gradle.PlatformUtils import com.wooga.gradle.test.PropertyQueryTaskWriter import com.wooga.gradle.test.TaskIntegrationSpec import com.wooga.gradle.test.writers.PropertyGetterTaskWriter -import com.wooga.gradle.test.writers.PropertySetterWriter import org.gradle.api.logging.LogLevel import spock.lang.Unroll import wooga.gradle.unity.testutils.GradleRunResult import wooga.gradle.unity.models.UnityCommandLineOption -import wooga.gradle.unity.tasks.Unity -import java.lang.reflect.ParameterizedType import java.nio.file.Files import java.nio.file.StandardCopyOption import java.time.Duration @@ -352,8 +349,10 @@ abstract class UnityTaskIntegrationSpec extends UnityIntegr @Unroll def "unity #message #maxRetries times with #retryWait wait times when line in log matches #retryRegexes"() { given: + def fakeUnity = createMockUnity(unityLog, 1) addUnityPathToExtension(fakeUnity.absolutePath) + buildFile << """ $subjectUnderTestName { maxRetries = ${wrapValue(maxRetries, Integer)} diff --git a/src/integrationTest/groovy/wooga/gradle/unity/tasks/FailTaskIntegrationSpec.groovy b/src/integrationTest/groovy/wooga/gradle/unity/tasks/FailTaskIntegrationSpec.groovy new file mode 100644 index 00000000..8aaa0868 --- /dev/null +++ b/src/integrationTest/groovy/wooga/gradle/unity/tasks/FailTaskIntegrationSpec.groovy @@ -0,0 +1,62 @@ +package wooga.gradle.unity.tasks + + +import com.wooga.spock.extensions.unity.UnityPluginTestOptions +import groovy.json.StringEscapeUtils +import wooga.gradle.unity.UnityTaskIntegrationSpec + +import java.util.regex.Matcher +import java.util.regex.Pattern + +class FailingTask extends Unity { + + static Pattern pattern = Pattern.compile("(?(.*\\s*)*)") + + // Escape for ^<, ^> + public static String expectedErrorMessage = """[BuildEngine] Collected errors during build process: + +[Error] FOO Wooga.UnifiedBuildSystem.Tests.Editor.BuildEngineTest+FailingBuildSteps:FailInElaborateWays (at Packages/com.wooga.unified-build-system/Tests/Editor/BuildEngine/BuildRequestOutputTest.cs:32) +[Error] BAR Wooga.UnifiedBuildSystem.Tests.Editor.BuildEngineTest+FailingBuildSteps:FailInElaborateWays (at Packages/com.wooga.unified-build-system/Tests/Editor/BuildEngine/BuildRequestOutputTest.cs:32) +[Exception] Exception: Boom! Wooga.UnifiedBuildSystem.Editor.BuildTask`1[[Wooga.UnifiedBuildSystem.Editor.BuildTaskArguments, Wooga.Build.Editor, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null]]:Execute (at Packages/com.wooga.build/Editor/Tasks/BuildTask.cs:73) +[Error] Failed to execute task 'FailInElaborateWays' Wooga.UnifiedBuildSystem.Editor.BuildEngine+State:SetBuildFailure (at Packages/com.wooga.unified-build-system/Editor/BuildEngine/BuildEngine.cs:89) +""".stripIndent() + + static String buildErrorStartMarker = "" + static String buildErrorEndMarker = "" + + @Override + protected String composeExceptionMessage(String stdout, String stderr) { + Matcher matcher = pattern.matcher(stderr) + if (matcher.find()){ + var summary = matcher.group(0) + return summary + } + return "" + } +} + +class FailTaskIntegrationSpec extends UnityTaskIntegrationSpec{ + + static String echoError(String message) { + """echo "${StringEscapeUtils.escapeJava(message).replace("`", "\\`") }" 1>&2 """ + } + + @UnityPluginTestOptions(writeMockExecutable = false) + def "throws composited exception message"() { + + given: + writeMockExecutable({ + it.text += "\n${FailingTask.expectedErrorMessage.readLines().collect{echoError(it) }.join("\n")}" + it.printEnvironment = false + it.exitValue = 666 + }) + + when: + def result = runTasksWithFailure(subjectUnderTestName) + + then: + result.wasExecuted(subjectUnderTestName) + result.standardError.contains(FailingTask.buildErrorStartMarker) + result.standardError.contains(FailingTask.buildErrorEndMarker) + } +} diff --git a/src/integrationTest/groovy/wooga/gradle/unity/testutils/GradleRunResult.groovy b/src/integrationTest/groovy/wooga/gradle/unity/testutils/GradleRunResult.groovy index 9037a9f3..bf2b2c88 100644 --- a/src/integrationTest/groovy/wooga/gradle/unity/testutils/GradleRunResult.groovy +++ b/src/integrationTest/groovy/wooga/gradle/unity/testutils/GradleRunResult.groovy @@ -1,5 +1,6 @@ package wooga.gradle.unity.testutils +import com.wooga.gradle.test.mock.MockExecutable import org.gradle.internal.impldep.org.apache.commons.lang.StringUtils class GradleRunResult { @@ -46,14 +47,14 @@ class GradleRunResult { } private static ArrayList loadArgs(String stdOutput) { - def argumentsStartToken = "[ARGUMENTS]:" + def argumentsStartToken = MockExecutable.ARGUMENTS_START_MARKER def lastExecutionOffset = stdOutput.lastIndexOf(argumentsStartToken) if(lastExecutionOffset < 0) { System.out.println(stdOutput) throw new IllegalArgumentException("couldn't find arguments list in stdout") } def lastExecTailString = stdOutput.substring(lastExecutionOffset) - def argsString = substringBetween(lastExecTailString, argumentsStartToken, "[LOG]"). + def argsString = substringBetween(lastExecTailString, argumentsStartToken, MockExecutable.ARGUMENTS_END_MARKER). replace(argumentsStartToken, "") def parts = argsString.split(" "). findAll {!StringUtils.isEmpty(it) }.collect{ it.trim() } @@ -61,8 +62,8 @@ class GradleRunResult { } private static Map loadEnvs(String stdOutput) { - String environmentStartToken = "[ENVIRONMENT]:" - def argsString = substringBetween(stdOutput, environmentStartToken, "[ARGUMENTS]"). + String environmentStartToken = MockExecutable.ENVIRONMENT_START_MARKER + def argsString = substringBetween(stdOutput, environmentStartToken, MockExecutable.ENVIRONMENT_END_MARKER). replace(environmentStartToken, "") def parts = argsString.split(System.lineSeparator()). findAll {!StringUtils.isEmpty(it) }.collect{ it.trim() } diff --git a/src/main/groovy/wooga/gradle/unity/UnityTask.groovy b/src/main/groovy/wooga/gradle/unity/UnityTask.groovy index 393baee0..5b45f8aa 100644 --- a/src/main/groovy/wooga/gradle/unity/UnityTask.groovy +++ b/src/main/groovy/wooga/gradle/unity/UnityTask.groovy @@ -98,19 +98,27 @@ abstract class UnityTask extends DefaultTask }) if (execResult.exitValue != 0) { - String message = "" - // Only write out the message if not already set to --info - if (!logger.infoEnabled) { - def stdout = logFile.text - def stderr = lastStderrStream? new String(lastStderrStream.toByteArray()) : "" - message = stderr ? stderr : stdout - } + def stdout = logFile.text + def stderr = lastStderrStream ? new String(lastStderrStream.toByteArray()) : "" + String message = composeExceptionMessage(stdout, stderr) throw new UnityExecutionException("Failed during execution of the Unity process with arguments:\n${_arguments}\n${message}") } postExecute(execResult) } + /** + * Composes the message to be included in an exception thrown when the task fails + * Override this in your custom task in order to improve reporting. + */ + protected String composeExceptionMessage(String stdout, String stderr) { + String message = "" + if (!logger.infoEnabled) { + message = stderr ? stderr : stdout + } + return message + } + /** * Invoked before the task executes the Unity process */