Add log error handling to Unity task when the Unity process execution fails

Author: Azurelol

src/integrationTest/groovy/wooga/gradle/unity/UnityPluginIntegrationSpec.groovy

@@ -20,7 +20,6 @@ package wooga.gradle.unity
 import com.wooga.spock.extensions.unity.UnityPathResolution
 import com.wooga.spock.extensions.unity.UnityPluginTestOptions
 import spock.lang.Unroll
-import wooga.gradle.unity.models.BuildTarget
 import wooga.gradle.unity.models.UnityCommandLineOption
 import wooga.gradle.unity.tasks.Test
 import wooga.gradle.unity.utils.ProjectSettingsFile

src/integrationTest/groovy/wooga/gradle/unity/UnityTaskIntegrationSpec.groovy

@@ -21,6 +21,7 @@ import com.wooga.spock.extensions.unity.UnityPathResolution
 import com.wooga.spock.extensions.unity.UnityPluginTestOptions
 import com.wooga.spock.extensions.uvm.UnityInstallation
 import org.gradle.api.logging.LogLevel
+import spock.lang.Ignore
 import spock.lang.IgnoreIf
 import spock.lang.Unroll
 import spock.util.environment.RestoreSystemProperties
@@ -406,4 +407,22 @@ abstract class UnityTaskIntegrationSpec<T extends UnityTask> extends UnityIntegr
         logFile.text.contains(mockUnityStartupMessage)
     }
 
+    @Ignore
+    // TODO: How to make the task fail/throw within the exec block?
+    def "task action does not invoke post execute if process didn't run"() {
+        given: "a task that is definitely gonna fail"
+        appendToSubjectTask("""
+                environment = null
+        """.stripIndent())
+
+        when:
+        def result = runTasks(subjectUnderTestName)
+
+        then:
+        !result.success
+        outputContains(result, "${subjectUnderTestName}.preExecute")
+        outputContains(result, "${subjectUnderTestName}.execute")
+        !outputContains(result, "${subjectUnderTestName}.postExecute")
+    }
+
 }

src/main/groovy/wooga/gradle/unity/UnityPlugin.groovy

@@ -31,16 +31,15 @@ import org.gradle.language.base.plugins.LifecycleBasePlugin
 import wooga.gradle.unity.models.APICompatibilityLevel
 import wooga.gradle.unity.models.DefaultUnityAuthentication
 import wooga.gradle.unity.internal.DefaultUnityPluginExtension
-import wooga.gradle.unity.models.BuildTarget
 import wooga.gradle.unity.models.TestPlatform
 import wooga.gradle.unity.tasks.Activate
 
 import wooga.gradle.unity.tasks.ReturnLicense
 import wooga.gradle.unity.tasks.SetAPICompatibilityLevel
 import wooga.gradle.unity.tasks.Test
 import wooga.gradle.unity.utils.ProjectSettingsFile
-import wooga.gradle.unity.utils.UnityTestTaskReport
-import wooga.gradle.unity.utils.UnityTestTaskReportsImpl
+import wooga.gradle.unity.utils.UnityLogErrorParse
+import wooga.gradle.unity.utils.UnityLogErrorReader
 
 /**
  * A {@link org.gradle.api.Plugin} which provides tasks to run unity batch-mode commands.
@@ -111,6 +110,7 @@ class UnityPlugin implements Plugin<Project> {
         extension.pluginsDir.convention(extension.assetsDir.dir("Plugins"))
         extension.logsDir.convention(UnityPluginConventions.logDirectory.getDirectoryValueProvider(project))
 
+        extension.logErrorReader.convention(new UnityLogErrorReader())
         extension.logCategory.convention(UnityPluginConventions.logCategory.getStringValueProvider(project))
         final ReportingExtension reportingExtension = (ReportingExtension) project.extensions.getByName(ReportingExtension.NAME)
         extension.reportsDir.convention(project.layout.buildDirectory.dir(project.provider({ reportingExtension.file("unity").path })))
@@ -149,6 +149,7 @@ class UnityPlugin implements Plugin<Project> {
             t.projectDirectory.convention(extension.projectDirectory)
             t.projectSettings.convention(extension.projectSettings)
             t.logCategory.convention(extension.logCategory)
+            t.logErrorReader.convention(extension.logErrorReader)
 
             t.unityLogFile.convention(extension.logsDir.file(project.provider {
                 t.logCategory.get().isEmpty() ? "${t.name}.log" : "${t.logCategory.get()}/${t.name}.log"

src/main/groovy/wooga/gradle/unity/UnityPluginExtension.groovy

@@ -24,7 +24,6 @@ import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.Internal
-import wooga.gradle.unity.models.BuildTarget
 import wooga.gradle.unity.traits.APICompatibilityLevelSpec
 import wooga.gradle.unity.traits.UnityAuthenticationSpec
 import wooga.gradle.unity.traits.UnityLicenseSpec

src/main/groovy/wooga/gradle/unity/UnityTask.groovy

@@ -27,13 +27,13 @@ import org.gradle.internal.io.LineBufferingOutputStream
 import org.gradle.internal.io.TextStream
 import org.gradle.process.ExecResult
 import org.gradle.process.ExecSpec
-import sun.reflect.misc.FieldUtil
 import wooga.gradle.unity.models.UnityCommandLineOption
 import wooga.gradle.unity.traits.ArgumentsSpec
 import wooga.gradle.unity.traits.UnityCommandLineSpec
 import wooga.gradle.unity.traits.UnitySpec
 import wooga.gradle.unity.utils.FileUtils
 import wooga.gradle.unity.utils.ForkTextStream
+import wooga.gradle.unity.utils.UnityLogErrorReader
 import wooga.gradle.unity.utils.UnityVersionManager
 
 abstract class UnityTask extends DefaultTask
@@ -47,21 +47,24 @@ abstract class UnityTask extends DefaultTask
         // and also an additional one for our custom use
         wooga_gradle_unity_traits_ArgumentsSpec__arguments = project.provider({ getUnityCommandLineOptions() })
         wooga_gradle_unity_traits_ArgumentsSpec__additionalArguments = project.objects.listProperty(String)
-        wooga_gradle_unity_traits_ArgumentsSpec__environment =  project.objects.mapProperty(String, Object)
+        wooga_gradle_unity_traits_ArgumentsSpec__environment = project.objects.mapProperty(String, Object)
     }
 
     @TaskAction
     void exec() {
+        // Invoked before the unity process
+        logger.info("${name}.preExecute")
+        preExecute()
+        // Execute the unity process
+        logger.info("${name}.execute")
         ExecResult execResult = project.exec(new Action<ExecSpec>() {
             @Override
             void execute(ExecSpec exec) {
 
+                // TODO: Should these be moved before preExecute?
                 if (!unityPath.present) {
                     throw new GradleException("Unity path is not set")
                 }
-
-                preExecute()
-
                 def unityPath = unityPath.get().asFile.absolutePath
                 def unityArgs = getAllArguments()
                 def unityEnvironment = environment.get()
@@ -76,8 +79,11 @@ abstract class UnityTask extends DefaultTask
                 }
             }
         })
-
-        execResult.assertNormalExitValue()
+        if (execResult.exitValue != 0) {
+            handleUnityProcessError(execResult)
+        }
+        // Invoked after the unity process (even if it failed)
+        logger.info("${name}.postExecute")
         postExecute(execResult)
     }
 
@@ -97,6 +103,31 @@ abstract class UnityTask extends DefaultTask
     protected void postExecute(ExecResult result) {
     }
 
+    /**
+     * Invoked whenever the Unity process executed by the task exits with an error,
+     * @param result The execution result of the Unity process
+     */
+    protected void handleUnityProcessError(ExecResult result) {
+        logger.error("Unity process failed with exit value ${result.exitValue}...")
+
+        // Look up the log
+        if (!unityLogFile.isPresent()) {
+            logger.warn("No log file was configured for the task ${this.name}")
+            return
+        }
+
+        File logFile = unityLogFile.get().asFile
+        if (!logFile.exists()) {
+            logger.warn("No log file was written for the task ${this.name}")
+            return
+        }
+
+        // TODO: Gracefully show the error here?
+        UnityLogErrorReader reader = new UnityLogErrorReader()
+        def errorParse = reader.parse(logFile)
+        logger.error(errorParse.toString())
+    }
+
     @Internal
     protected ArtifactVersion getUnityVersion() {
         File file = unityPath.present ? unityPath.get().asFile : null

src/main/groovy/wooga/gradle/unity/traits/UnitySpec.groovy

@@ -16,25 +16,17 @@
 
 package wooga.gradle.unity.traits
 
-import org.gradle.api.model.ObjectFactory
 import org.gradle.api.file.Directory
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.RegularFile
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
-import org.gradle.api.provider.ProviderFactory
-import org.gradle.api.tasks.Input
-import org.gradle.api.tasks.InputDirectory
 import org.gradle.api.tasks.InputFile
 import org.gradle.api.tasks.Internal
-import org.gradle.api.tasks.Optional
 import org.gradle.api.tasks.OutputFile
-import org.gradle.api.tasks.SkipWhenEmpty
-import org.gradle.internal.impldep.org.eclipse.jgit.errors.NotSupportedException
-import wooga.gradle.unity.UnityPluginConventions
-import wooga.gradle.unity.models.BuildTarget
 import wooga.gradle.unity.utils.ProjectSettingsFile
+import wooga.gradle.unity.utils.UnityLogErrorReader
 
 import javax.inject.Inject
 
@@ -115,5 +107,14 @@ trait UnitySpec extends UnityBaseSpec {
         logToStdout
     }
 
+    private final Property<UnityLogErrorReader> logErrorReader = objects.property(UnityLogErrorReader)
+    /**
+     * @return The reader used for parsing the Unity Editor log whenever the exits with a failure code
+     */
+    @Internal
+    Property<UnityLogErrorReader> getLogErrorReader(){
+        logErrorReader
+    }
+
 }