From 9d0c4829fc1d8e40c0b9349136001810d178d6d1 Mon Sep 17 00:00:00 2001 From: Alexander Botkin Date: Mon, 11 Nov 2019 11:35:56 -0800 Subject: [PATCH] =?UTF-8?q?[BUG]=20xcparse=20doesn't=20export=20screenshot?= =?UTF-8?q?s=20for=20iPhone=20X=CA=80=20on=20Xcode=2011.1=20or=20below=20(?= =?UTF-8?q?#31)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change Description: These changes are to help address issues with using xcparse on Xcode 11.1 and below. Apple's xcresulttool, which we use to extract the screenshots, crashes in some versions when attempting to export out attachments to directory paths that have Unicode characters like "iPhone Xʀ" or "한국어". This leads to the user being unable to get their screenshots & not knowing why. The changes here introduce the ability for xcparse to understand the version of xcresulttool & change behavior of exporting based off the versioning. For xcresulttool versions below 15500, we change "iPhone Xʀ" to "iPhone XR" & any other non-ASCII compatible characters into their lossy ASCII conversion (often "?"). For xcresulttool 15500 or above, we continue exporting how we always have. In all cases for users with xcresulttool below 15500, we warn them about the issue in their version and encourage them to update Xcode. In cases where users with xcresulttool version below 155000 have a destination folder path that can not be represented in ASCII, we hard-fail export and alert the user they need to update Xcode. Test Plan/Testing Performed: Tested that with these changes, iPhone XR screenshots can be retrieved on Xcode 11.1. When using test run configuration names with Korean characters, confirmed that we export into a lossy ASCII version of the string (though this will lead to loss of some folders since "한국어" & "중국어" will both become "???") --- Sources/XCParseCore/Version+XCPTooling.swift | 47 ++++++++++ Sources/XCParseCore/XCResultToolCommand.swift | 12 +++ Sources/xcparse/AttachmentsCommand.swift | 2 + Sources/xcparse/CommandRegistry.swift | 7 +- Sources/xcparse/ScreenshotsCommand.swift | 2 + Sources/xcparse/VersionCommand.swift | 1 + Sources/xcparse/XCPParser.swift | 85 +++++++++++++++++-- xcparse.xcodeproj/project.pbxproj | 4 + 8 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 Sources/XCParseCore/Version+XCPTooling.swift diff --git a/Sources/XCParseCore/Version+XCPTooling.swift b/Sources/XCParseCore/Version+XCPTooling.swift new file mode 100644 index 0000000..d726534 --- /dev/null +++ b/Sources/XCParseCore/Version+XCPTooling.swift @@ -0,0 +1,47 @@ +// +// Version+XCPTooling.swift +// XCParseCore +// +// Created by Alex Botkin on 11/8/19. +// + +import Foundation +import SPMUtility + +public extension Version { + static func xcresulttoolCompatibleWithUnicodeExportPath() -> Version { + return Version(15500, 0, 0) + } + + static func xcresulttool() -> Version? { + guard let xcresulttoolVersionResult = XCResultToolCommand.Version().run() else { + return nil + } + do { + let xcresultVersionString = try xcresulttoolVersionResult.utf8Output() + + let components = xcresultVersionString.components(separatedBy: CharacterSet(charactersIn: ",\n")) + for string in components { + let trimmedString = string.trimmingCharacters(in: .whitespacesAndNewlines) + if trimmedString.hasPrefix("xcresulttool version ") { + let xcresulttoolVersionString = trimmedString.replacingOccurrences(of: "xcresulttool version ", with: "") + // Check to see if we can convert it to a number + var xcresulttoolVersion: Version? + + if let xcresulttoolVersionInt = Int(xcresulttoolVersionString) { + xcresulttoolVersion = Version(xcresulttoolVersionInt, 0, 0) + } else { + xcresulttoolVersion = Version(string: xcresulttoolVersionString) + } + + return xcresulttoolVersion + } + } + + return nil + } catch { + print("Failed to parse xcresulttool version with error: \(error)") + return nil + } + } +} diff --git a/Sources/XCParseCore/XCResultToolCommand.swift b/Sources/XCParseCore/XCResultToolCommand.swift index bafd038..433dec0 100644 --- a/Sources/XCParseCore/XCResultToolCommand.swift +++ b/Sources/XCParseCore/XCResultToolCommand.swift @@ -167,4 +167,16 @@ open class XCResultToolCommand { super.init(withXCResult: xcresult, process: process) } } + + open class Version: XCResultToolCommand { + + public init() { + var processArgs = xcresultToolArguments + processArgs.append(contentsOf: ["version"]) + + let xcresult = XCResult(path: "") + let process = Basic.Process(arguments: processArgs) + super.init(withXCResult: xcresult, process: process) + } + } } diff --git a/Sources/xcparse/AttachmentsCommand.swift b/Sources/xcparse/AttachmentsCommand.swift index 59d772d..0b4e2d0 100644 --- a/Sources/xcparse/AttachmentsCommand.swift +++ b/Sources/xcparse/AttachmentsCommand.swift @@ -97,6 +97,8 @@ struct AttachmentsCommand: Command { options.activitySummaryFilter = { additionalActivityTypes.contains($0.activityType) } } + options.xcresulttoolCompatability = xcpParser.checkXCResultToolCompatability(destination: outputPath.pathString) + // Now let's get extracting try xcpParser.extractAttachments(xcresultPath: xcresultPath.pathString, destination: outputPath.pathString, diff --git a/Sources/xcparse/CommandRegistry.swift b/Sources/xcparse/CommandRegistry.swift index fe0f375..6941ec7 100644 --- a/Sources/xcparse/CommandRegistry.swift +++ b/Sources/xcparse/CommandRegistry.swift @@ -60,15 +60,20 @@ struct CommandRegistry { do { let xcpParser = XCPParser() + + let destination = legacyScreenshotPaths[1].path.pathString + let xcresulttoolCompatability = xcpParser.checkXCResultToolCompatability(destination: destination) + let options = AttachmentExportOptions(addTestScreenshotsDirectory: true, divideByTargetModel: false, divideByTargetOS: false, divideByTestRun: false, + xcresulttoolCompatability: xcresulttoolCompatability, attachmentFilter: { return UTTypeConformsTo($0.uniformTypeIdentifier as CFString, "public.image" as CFString) }) try xcpParser.extractAttachments(xcresultPath: legacyScreenshotPaths[0].path.pathString, - destination: legacyScreenshotPaths[1].path.pathString, + destination: destination, options: options) return true diff --git a/Sources/xcparse/ScreenshotsCommand.swift b/Sources/xcparse/ScreenshotsCommand.swift index c2640e6..83ac4b5 100644 --- a/Sources/xcparse/ScreenshotsCommand.swift +++ b/Sources/xcparse/ScreenshotsCommand.swift @@ -93,6 +93,8 @@ struct ScreenshotsCommand: Command { options.activitySummaryFilter = { additionalActivityTypes.contains($0.activityType) } } + options.xcresulttoolCompatability = xcpParser.checkXCResultToolCompatability(destination: outputPath.pathString) + try xcpParser.extractAttachments(xcresultPath: xcresultPath.pathString, destination: outputPath.pathString, options: options) diff --git a/Sources/xcparse/VersionCommand.swift b/Sources/xcparse/VersionCommand.swift index 85518f2..4861786 100644 --- a/Sources/xcparse/VersionCommand.swift +++ b/Sources/xcparse/VersionCommand.swift @@ -9,6 +9,7 @@ import Basic import Foundation import SPMUtility +import XCParseCore struct VersionCommand: Command { let command = "version" diff --git a/Sources/xcparse/XCPParser.swift b/Sources/xcparse/XCPParser.swift index 49b5910..2a179fb 100644 --- a/Sources/xcparse/XCPParser.swift +++ b/Sources/xcparse/XCPParser.swift @@ -11,7 +11,7 @@ import Foundation import SPMUtility import XCParseCore -let xcparseCurrentVersion = Version(1, 0, 0) +let xcparseCurrentVersion = Version(1, 0, 1) extension Foundation.URL { func fileExistsAsDirectory() -> Bool { @@ -49,6 +49,24 @@ extension Foundation.URL { } } +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 +} + struct AttachmentExportOptions { var addTestScreenshotsDirectory: Bool = false var divideByTargetModel: Bool = false @@ -56,6 +74,8 @@ struct AttachmentExportOptions { var divideByTestRun: Bool = false var divideByTest: Bool = false + var xcresulttoolCompatability = XCResultToolCompatability() + var testSummaryFilter: (ActionTestSummary) -> Bool = { _ in return true } @@ -78,16 +98,27 @@ struct AttachmentExportOptions { func screenshotDirectoryURL(_ deviceRecord: ActionDeviceRecord, forBaseURL baseURL: Foundation.URL) -> Foundation.URL { var targetDeviceFolderName: String? = nil + var modelName = deviceRecord.modelName + if self.xcresulttoolCompatability.supportsUnicodeExportPaths != true, modelName == "iPhone Xʀ" { + // For explaination, see https://github.com/ChargePoint/xcparse/issues/30 + modelName = "iPhone XR" + } + if self.divideByTargetModel == true, self.divideByTargetOS == true { - targetDeviceFolderName = deviceRecord.modelName + " (\(deviceRecord.operatingSystemVersion))" + targetDeviceFolderName = modelName + " (\(deviceRecord.operatingSystemVersion))" } else if self.divideByTargetModel { - targetDeviceFolderName = deviceRecord.modelName + targetDeviceFolderName = modelName } else if self.divideByTargetOS { targetDeviceFolderName = deviceRecord.operatingSystemVersion } if let folderName = targetDeviceFolderName { - return baseURL.appendingPathComponent(folderName, isDirectory: true) + if self.xcresulttoolCompatability.supportsUnicodeExportPaths != true { + let asciiFolderName = folderName.lossyASCIIString() ?? folderName + return baseURL.appendingPathComponent(asciiFolderName, isDirectory: true) + } else { + return baseURL.appendingPathComponent(folderName, isDirectory: true) + } } else { return baseURL } @@ -99,7 +130,12 @@ struct AttachmentExportOptions { } if self.divideByTestRun { - return baseURL.appendingPathComponent(testPlanRunName, isDirectory: true) + if self.xcresulttoolCompatability.supportsUnicodeExportPaths != true { + let asciiTestPlanRunName = testPlanRunName.lossyASCIIString() ?? testPlanRunName + return baseURL.appendingPathComponent(asciiTestPlanRunName, isDirectory: true) + } else { + return baseURL.appendingPathComponent(testPlanRunName, isDirectory: true) + } } else { return baseURL } @@ -111,7 +147,12 @@ struct AttachmentExportOptions { } if self.divideByTest == true { - return baseURL.appendingPathComponent(summaryIdentifier, isDirectory: true) + if self.xcresulttoolCompatability.supportsUnicodeExportPaths != true { + let asciiSummaryIdentifier = summaryIdentifier.lossyASCIIString() ?? summaryIdentifier + return baseURL.appendingPathComponent(asciiSummaryIdentifier, isDirectory: true) + } else { + return baseURL.appendingPathComponent(summaryIdentifier, isDirectory: true) + } } else { return baseURL } @@ -127,7 +168,39 @@ class XCPParser { // MARK: - // MARK: Parsing Actions + func checkXCResultToolCompatability(destination: String) -> XCResultToolCompatability { + var compatability = XCResultToolCompatability() + + guard let xcresulttoolVersion = Version.xcresulttool() else { + self.console.writeMessage("Warning: Could not determine xcresulttool version", to: .standard) + return compatability + } + + let unicodeExport = Version.xcresulttoolCompatibleWithUnicodeExportPath() + if xcresulttoolVersion < unicodeExport { + // For explaination, see https://github.com/ChargePoint/xcparse/issues/30 + let asciiDestinationPath = destination.lossyASCIIString() ?? destination + if asciiDestinationPath != destination { + self.console.writeMessage("\nYour xcresulttool version \(xcresulttoolVersion.major) does not fully support Unicode export directory paths. Upgrade to Xcode 11.2.1 (xcresulttool version \(unicodeExport.major)) in order to export to your non-ASCII destination path.\n", to: .standard) + + compatability.supportsExport = false + compatability.supportsUnicodeExportPaths = false + } else { + self.console.writeMessage("\nYour xcresulttool version \(xcresulttoolVersion.major) does not fully support Unicode export directory paths. Upgrade to Xcode 11.2.1 (xcresulttool version \(unicodeExport.major)) or above if you use non-Latin characters in your test run configuration names, attachment file names, or file system folder names.\n", to: .standard) + + compatability.supportsUnicodeExportPaths = false + } + } + + return compatability + } + func extractAttachments(xcresultPath: String, destination: String, options: AttachmentExportOptions = AttachmentExportOptions()) throws { + // Check the xcresulttool version is compatible to export the request + if options.xcresulttoolCompatability.supportsExport != true { + return + } + var xcresult = XCResult(path: xcresultPath, console: self.console) guard let invocationRecord = xcresult.invocationRecord else { return diff --git a/xcparse.xcodeproj/project.pbxproj b/xcparse.xcodeproj/project.pbxproj index e797c53..21149dd 100644 --- a/xcparse.xcodeproj/project.pbxproj +++ b/xcparse.xcodeproj/project.pbxproj @@ -21,6 +21,7 @@ /* End PBXAggregateTarget section */ /* Begin PBXBuildFile section */ + 62525EAE2376350100472F82 /* Version+XCPTooling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62525EAD2376350100472F82 /* Version+XCPTooling.swift */; }; 62CC363E23553EA0003C7B68 /* XCResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62CC363D23553EA0003C7B68 /* XCResult.swift */; }; 62CC36592357C110003C7B68 /* AttachmentsCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62CC36582357C110003C7B68 /* AttachmentsCommand.swift */; }; OBJ_179 /* Await.swift in Sources */ = {isa = PBXBuildFile; fileRef = OBJ_73 /* Await.swift */; }; @@ -327,6 +328,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 62525EAD2376350100472F82 /* Version+XCPTooling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Version+XCPTooling.swift"; sourceTree = ""; }; 62CC363D23553EA0003C7B68 /* XCResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XCResult.swift; sourceTree = ""; }; 62CC36582357C110003C7B68 /* AttachmentsCommand.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttachmentsCommand.swift; sourceTree = ""; }; OBJ_10 /* ActionDeviceRecord.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ActionDeviceRecord.swift; sourceTree = ""; }; @@ -945,6 +947,7 @@ OBJ_52 /* TestFailureIssueSummary.swift */, OBJ_53 /* TypeDefinition.swift */, OBJ_54 /* XCPResultDecoding.swift */, + 62525EAD2376350100472F82 /* Version+XCPTooling.swift */, OBJ_55 /* XCResultToolCommand.swift */, 62CC363D23553EA0003C7B68 /* XCResult.swift */, ); @@ -1267,6 +1270,7 @@ OBJ_295 /* ActionTestSummaryIdentifiableObject.swift in Sources */, OBJ_296 /* ActionTestableSummary.swift in Sources */, OBJ_297 /* ActionsInvocationMetadata.swift in Sources */, + 62525EAE2376350100472F82 /* Version+XCPTooling.swift in Sources */, OBJ_298 /* ActionsInvocationRecord.swift in Sources */, OBJ_299 /* ActivityLogAnalyzerControlFlowStep.swift in Sources */, OBJ_300 /* ActivityLogAnalyzerControlFlowStepEdge.swift in Sources */,