xcparse should give better error when xcresult path leads to non-xcre…

…sult (#47)

Change Description: This fixes #40 by putting an explicit check to see if the xcresult path exists as a directory with an Info.plist before attempting any xcresulttool attempts. If the path does not pass, an error is given to the user highlighting the input path they gave that does not appear to be an xcresult.

Test Plan/Testing Performed: Added unit tests to the commands to confirm the error will get shown.

Sources/XCParseCore/ActionsInvocationRecord.swift

@@ -35,6 +35,20 @@ public class ActionsInvocationRecord : Codable {
     }
 
     class public func recordFromXCResult(_ xcresult: XCResult) -> ActionsInvocationRecord? {
+        let xcresultURL = URL(fileURLWithPath: xcresult.path)
+        if xcresultURL.fileExistsAsDirectory() == false {
+            // .xcresult is a directory structure, if yours is a file perhaps someone gave you a zip
+            // and didn't attach the file extension?
+            //
+            // See: https://github.com/bitrise-steplib/steps-create-zip/issues/7
+            return nil
+        }
+        let xcresultPlistURL = xcresultURL.appendingPathComponent("Info.plist")
+        if FileManager.default.fileExists(atPath: xcresultPlistURL.path) == false {
+            // .xcresult should have an Info.plist in both pre-Xcode 11 and post-Xcode 11
+            return nil
+        }
+
         guard let xcresultGetResult = XCResultToolCommand.Get(withXCResult: xcresult, id: "", outputPath: "", format: .json).run() else {
             return nil
         }

Sources/XCParseCore/Extensions/String+ASCII.swift

@@ -0,0 +1,21 @@
+//
+//  String+ASCII.swift
+//  
+//
+//  Created by Alexander Botkin on 7/5/20.
+//
+
+import Foundation
+
+public extension String {
+    func lossyASCIIString() -> String? {
+        let string = self.precomposedStringWithCanonicalMapping
+        guard let lossyASCIIData = string.data(using: .ascii, allowLossyConversion: true) else {
+            return nil
+        }
+        guard let lossyASCIIString = String(data: lossyASCIIData, encoding: .ascii) else {
+            return nil
+        }
+        return lossyASCIIString
+    }
+}

Sources/XCParseCore/Extensions/URL+Directory.swift

@@ -0,0 +1,44 @@
+//
+//  URL+Directory.swift
+//  
+//
+//  Created by Alexander Botkin on 7/5/20.
+//
+
+import Foundation
+
+public extension Foundation.URL {
+    func fileExistsAsDirectory() -> Bool {
+        var isDirectory: ObjCBool = false
+        if FileManager.default.fileExists(atPath: self.path, isDirectory: &isDirectory) {
+            if isDirectory.boolValue {
+                return true // Exists as a directory
+            } else {
+                return false // Exists as a file
+            }
+        } else {
+            return false // Does not exist
+        }
+    }
+
+    func createDirectoryIfNecessary(createIntermediates: Bool = false, console: Console = Console()) -> Bool {
+        var isDirectory: ObjCBool = false
+        if FileManager.default.fileExists(atPath: self.path, isDirectory: &isDirectory) {
+            if isDirectory.boolValue {
+                // Directory already exists, do nothing
+                return true
+            } else {
+                console.writeMessage("\(self) is not a directory", to: .error)
+                return false
+            }
+        } else {
+            if createIntermediates == true {
+                console.shellCommand(["mkdir", "-p", self.path])
+            } else {
+                console.shellCommand(["mkdir", self.path])
+            }
+        }
+
+        return self.fileExistsAsDirectory()
+    }
+}

Sources/xcparse/XCPParser.swift

@@ -13,55 +13,6 @@ import XCParseCore
 
 let xcparseCurrentVersion = Version(2, 1, 0)
 
-extension Foundation.URL {
-    func fileExistsAsDirectory() -> Bool {
-        var isDirectory: ObjCBool = false
-        if FileManager.default.fileExists(atPath: self.path, isDirectory: &isDirectory) {
-            if isDirectory.boolValue {
-                return true // Exists as a directory
-            } else {
-                return false // Exists as a file
-            }
-        } else {
-            return false // Does not exist
-        }
-    }
-
-    func createDirectoryIfNecessary(createIntermediates: Bool = false, console: Console = Console()) -> Bool {
-        var isDirectory: ObjCBool = false
-        if FileManager.default.fileExists(atPath: self.path, isDirectory: &isDirectory) {
-            if isDirectory.boolValue {
-                // Directory already exists, do nothing
-                return true
-            } else {
-                console.writeMessage("\(self) is not a directory", to: .error)
-                return false
-            }
-        } else {
-            if createIntermediates == true {
-                console.shellCommand(["mkdir", "-p", self.path])
-            } else {
-                console.shellCommand(["mkdir", self.path])
-            }
-        }
-
-        return self.fileExistsAsDirectory()
-    }
-}
-
-extension String {
-    func lossyASCIIString() -> String? {
-        let string = self.precomposedStringWithCanonicalMapping
-        guard let lossyASCIIData = string.data(using: .ascii, allowLossyConversion: true) else {
-            return nil
-        }
-        guard let lossyASCIIString = String(data: lossyASCIIData, encoding: .ascii) else {
-            return nil
-        }
-        return lossyASCIIString
-    }
-}
-
 struct XCResultToolCompatability {
     var supportsExport: Bool = true
     var supportsUnicodeExportPaths: Bool = true // See https://github.com/ChargePoint/xcparse/issues/30
@@ -230,6 +181,7 @@ class XCPParser {
 
         var xcresult = XCResult(path: xcresultPath, console: self.console)
         guard let invocationRecord = xcresult.invocationRecord else {
+            xcresult.console.writeMessage("“\(xcresult.path)” does not appear to be an xcresult", to: .error)
             return
         }
 
@@ -336,6 +288,7 @@ class XCPParser {
     func extractCoverage(xcresultPath : String, destination : String) throws {
         var xcresult = XCResult(path: xcresultPath, console: self.console)
         guard let invocationRecord = xcresult.invocationRecord else {
+            xcresult.console.writeMessage("“\(xcresult.path)” does not appear to be an xcresult", to: .error)
             return
         }
 
@@ -369,6 +322,7 @@ class XCPParser {
     func extractLogs(xcresultPath : String, destination : String) throws {
         var xcresult = XCResult(path: xcresultPath, console: self.console)
         guard let invocationRecord = xcresult.invocationRecord else {
+            xcresult.console.writeMessage("“\(xcresult.path)” does not appear to be an xcresult", to: .error)
             return
         }
 

Tests/xcparseTests/xcparseTests.swift

@@ -42,9 +42,14 @@ final class xcparseTests: XCTestCase {
         ("testDivideByTest",testDivideByTest),
         ("testGetTestsWithFailure",testGetTestsWithFailure),
         ("testScreenshotsHEIC", testScreenshotsHEIC),
+        ("testScreenshotsMissingInput", testScreenshotsMissingInput),
         ("testGetCodeCoverage",testGetCodeCoverage),
+        ("testCodeCoverageMissingInput",testCodeCoverageMissingInput),
         ("testGetLogs",testGetLogs),
+        ("testLogsMissingInput",testLogsMissingInput),
         ("testDivideAttachmentsWithUTIFlags",testDivideAttachmentsWithUTIFlags),
+        ("testAttachmentsHEIC",testAttachmentsHEIC),
+        ("testAttachmentsMissingInput",testAttachmentsMissingInput),
     ]
 
     func runAndWaitForXCParseProcess() throws  {
@@ -260,6 +265,24 @@ final class xcparseTests: XCTestCase {
         XCTAssertTrue(heicURLs.count == 18)
     }
 
+    func testScreenshotsMissingInput() throws {
+        guard #available(macOS 10.13, *) else {
+            return
+        }
+
+        xcparseProcess.arguments = ["screenshots", "/tmp", temporaryOutputDirectoryURL.path]
+
+        let pipe = Pipe()
+        xcparseProcess.standardError = pipe
+
+        try runAndWaitForXCParseProcess()
+
+        let data = pipe.fileHandleForReading.readDataToEndOfFile()
+        let output = String(data: data, encoding: .utf8)
+
+        XCTAssertEqual(output, "Error: “/tmp” does not appear to be an xcresult\n")
+    }
+
     // MARK: - Command - Code Coverage
 
     func testGetCodeCoverage() throws {
@@ -281,6 +304,24 @@ final class xcparseTests: XCTestCase {
 
     }
 
+    func testCodeCoverageMissingInput() throws {
+        guard #available(macOS 10.13, *) else {
+            return
+        }
+
+        xcparseProcess.arguments = ["codecov", "/tmp", temporaryOutputDirectoryURL.path]
+
+        let pipe = Pipe()
+        xcparseProcess.standardError = pipe
+
+        try runAndWaitForXCParseProcess()
+
+        let data = pipe.fileHandleForReading.readDataToEndOfFile()
+        let output = String(data: data, encoding: .utf8)
+
+        XCTAssertEqual(output, "Error: “/tmp” does not appear to be an xcresult\n")
+    }
+
     // MARK: - Command - Logs
 
     func testGetLogs() throws {
@@ -299,6 +340,24 @@ final class xcparseTests: XCTestCase {
         XCTAssertTrue(fileUrls.count > 0)
     }
 
+    func testLogsMissingInput() throws {
+        guard #available(macOS 10.13, *) else {
+            return
+        }
+
+        xcparseProcess.arguments = ["logs", "/tmp", temporaryOutputDirectoryURL.path]
+
+        let pipe = Pipe()
+        xcparseProcess.standardError = pipe
+
+        try runAndWaitForXCParseProcess()
+
+        let data = pipe.fileHandleForReading.readDataToEndOfFile()
+        let output = String(data: data, encoding: .utf8)
+
+        XCTAssertEqual(output, "Error: “/tmp” does not appear to be an xcresult\n")
+    }
+
     // MARK: - Command - Attachments
 
     func testDivideAttachmentsWithUTIFlags() throws {
@@ -328,4 +387,22 @@ final class xcparseTests: XCTestCase {
         let heicURLs = fileURLs.filter { $0.pathExtension == "heic" }
         XCTAssertTrue(heicURLs.count == 18)
     }
+
+    func testAttachmentsMissingInput() throws {
+        guard #available(macOS 10.13, *) else {
+            return
+        }
+
+        xcparseProcess.arguments = ["attachments", "/tmp", temporaryOutputDirectoryURL.path]
+
+        let pipe = Pipe()
+        xcparseProcess.standardError = pipe
+
+        try runAndWaitForXCParseProcess()
+
+        let data = pipe.fileHandleForReading.readDataToEndOfFile()
+        let output = String(data: data, encoding: .utf8)
+
+        XCTAssertEqual(output, "Error: “/tmp” does not appear to be an xcresult\n")
+    }
 }