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

… fails

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
@@ -386,4 +387,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,13 @@ 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
 
 /**
  * A {@link org.gradle.api.Plugin} which provides tasks to run unity batch-mode commands.

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,15 @@ import org.gradle.internal.io.LineBufferingOutputStream
 import org.gradle.internal.io.TextStream
 import org.gradle.process.ExecResult
 import org.gradle.process.ExecSpec
+import org.gradle.api.file.RegularFile
 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 +49,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 +81,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 +105,30 @@ 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?
+        def errorParse = UnityLogErrorReader.readErrorMessageFromLog(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,24 +16,15 @@
 
 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 javax.inject.Inject

src/main/groovy/wooga/gradle/unity/utils/UnityLogErrorReader.groovy

@@ -17,37 +17,166 @@
 
 package wooga.gradle.unity.utils
 
+import java.util.regex.Matcher
+
+enum UnityLogErrorType {
+    /**
+     * No error was found
+     */
+    None,
+    /**
+     * The error could not be parsed
+     */
+    Unknown,
+    /**
+     * The Unity scripts have compiler errors
+     */
+    ScriptCompilerError,
+    /**
+     * The build failed
+     */
+    BuildFailure,
+    /**
+     * Tests failed
+     */
+    TestFailure
+}
+
+class UnityLogErrorParse {
+    UnityLogErrorType type
+    String message
+    List<CSharpFileCompilationResult> compilerOutput
+
+    @Override
+    String toString() {
+        def result = "${type}:"
+        switch (type) {
+            case UnityLogErrorType.ScriptCompilerError:
+                compilerOutput.forEach({ l ->
+                    result += "\n${l.text}"
+                })
+                break
+        }
+        result
+    }
+}
+
+class CSharpFileCompilationResult {
+    String text
+    String filePath
+    Integer line
+    Integer column
+    String level
+    String code
+    String message
+
+    CSharpFileCompilationResult(String filePath, Integer line, Integer column, String level, String code, String message) {
+        this.filePath = filePath
+        this.line = line
+        this.column = column
+        this.level = level
+        this.code = code
+        this.message = message
+    }
+
+    CSharpFileCompilationResult(String text, String filePath, Integer line, Integer column, String level, String code, String message) {
+        this.text = text
+        this.filePath = filePath
+        this.line = line
+        this.column = column
+        this.level = level
+        this.code = code
+        this.message = message
+    }
+
+    CSharpFileCompilationResult() {
+    }
+
+    // e.g: "A/B.cs(9,7): warning CS0105: WRONG!"
+    static String pattern = /(?<filePath>.+)\((?<line>\d+),(?<column>\d+)\):\s(?<level>.*?)\s(?<code>.+):\s(?<message>.*)/
+
+    static CSharpFileCompilationResult Parse(String text) {
+        Matcher matcher = (text =~ pattern)
+        if (matcher.count == 0) {
+            return null
+        }
+
+        def (all, filePath, line, column, level, code, message) = matcher[0]
+        CSharpFileCompilationResult result = new CSharpFileCompilationResult()
+        result.text = all
+        result.filePath = filePath
+        result.line = Integer.parseInt(line)
+        result.column = Integer.parseInt(column)
+        result.level = level
+        result.code = code
+        result.message = message
+        result
+    }
+}
+
 class UnityLogErrorReader {
 
-    static String DEFAULT_MESSAGE = "no error"
+    static String compilerOutputStartMarker = "-----CompilerOutput:"
+    static String compilerOutputEndMarker = "-----EndCompilerOutput"
+    static String compilerErrorMarker = "Scripts have compiler errors."
+    static String errorMarker = "Aborting batchmode due to failure:"
+    static String dialogError = "Cancelling DisplayDialog:"
 
-    static String  readErrorMessageFromLog(File logfile) {
-        def message = DEFAULT_MESSAGE
-        if(!logfile || !logfile.exists()) {
-            return message
+    static UnityLogErrorParse readErrorMessageFromLog(File logfile) {
+
+        UnityLogErrorParse parse = new UnityLogErrorParse()
+        parse.type = UnityLogErrorType.None
+
+        if (!logfile || !logfile.exists()) {
+            return parse
         }
 
+        boolean foundCompilerMarker = false
         boolean foundErrorMarker = false
-        String errorMarker = "Aborting batchmode due to failure:"
-        String dialogError = "Cancelling DisplayDialog:"
+        parse.compilerOutput = []
         String line
+        Integer lineNumber = 0
+
+        // Read through the log file
         logfile.withReader { reader ->
-            while ((line = reader.readLine())!=null) {
-                if(foundErrorMarker) {
-                    message = line
-                    break
-                }
-                if(line.startsWith(errorMarker)) {
-                    foundErrorMarker = true
-                }
+            while ((line = reader.readLine()) != null) {
+                lineNumber++
 
-                if(line.startsWith(dialogError)) {
-                    message = line.replace(dialogError, "").trim()
-                    break
+                // If inside a compiler marker, parse the compiler output
+                if (foundCompilerMarker) {
+                    // Finished reading compiler output
+                    if (line.startsWith(compilerOutputEndMarker)) {
+                        foundCompilerMarker = false
+                    }
+                    // Record all warnings/errors
+                    else {
+                        CSharpFileCompilationResult fileCompilationResult = CSharpFileCompilationResult.Parse(line)
+                        if (fileCompilationResult != null) {
+                            parse.compilerOutput.add(fileCompilationResult)
+                        }
+                    }
+                } else if (foundErrorMarker) {
+                    if (line.startsWith(compilerErrorMarker)) {
+                        parse.type = UnityLogErrorType.ScriptCompilerError
+                        break
+                    } else {
+                        parse.message = line
+                    }
+                }
+                // Look for markers
+                else {
+                    // The error marker is found near the end of the log output
+                    if (line.startsWith(errorMarker)) {
+                        parse.type = UnityLogErrorType.Unknown
+                        foundErrorMarker = true
+                    }
+                    // Started reading through C# compiler output
+                    else if (line.startsWith(compilerOutputStartMarker)) {
+                        foundCompilerMarker = true
+                    }
                 }
             }
         }
-
-        message
+        parse
     }
 }