-
Notifications
You must be signed in to change notification settings - Fork 220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Actually fail CI when tests fail #313
Changes from all commits
2c9e006
ad1eb3b
b581789
e8ddf17
0a38a23
f6a4def
59883b2
d2651be
aa31459
30c4016
f69a5fe
058c73e
49f3470
a0bb5e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,131 +21,75 @@ enum TaskError: Error { | |
} | ||
|
||
enum Platform: String, CustomStringConvertible { | ||
case iOS_13 | ||
case iOS_14 | ||
case iOS_15 | ||
case iOS_16 | ||
case iOS_17 | ||
case tvOS_13 | ||
case tvOS_14 | ||
case tvOS_15 | ||
case tvOS_16 | ||
case tvOS_17 | ||
case macOS_11 | ||
case macOS_12 | ||
case macOS_13 | ||
case macOS_14 | ||
case watchOS_6 | ||
case watchOS_7 | ||
case watchOS_8 | ||
case watchOS_9 | ||
case watchOS_10 | ||
|
||
var destination: String { | ||
switch self { | ||
case .iOS_13: | ||
return "platform=iOS Simulator,OS=13.7,name=iPad Pro (12.9-inch) (4th generation)" | ||
case .iOS_14: | ||
return "platform=iOS Simulator,OS=14.4,name=iPad Pro (12.9-inch) (4th generation)" | ||
case .iOS_15: | ||
return "platform=iOS Simulator,OS=15.5,name=iPad Pro (12.9-inch) (5th generation)" | ||
case .iOS_16: | ||
return "platform=iOS Simulator,OS=16.4,name=iPad Pro (12.9-inch) (6th generation)" | ||
case .iOS_17: | ||
return "platform=iOS Simulator,OS=17.4,name=iPad Pro (12.9-inch) (6th generation)" | ||
|
||
case .tvOS_13: | ||
return "platform=tvOS Simulator,OS=13.4,name=Apple TV" | ||
case .tvOS_14: | ||
return "platform=tvOS Simulator,OS=14.3,name=Apple TV" | ||
case .tvOS_15: | ||
return "platform=tvOS Simulator,OS=15.4,name=Apple TV" | ||
case .tvOS_16: | ||
return "platform=tvOS Simulator,OS=16.4,name=Apple TV" | ||
case .tvOS_17: | ||
return "platform=tvOS Simulator,OS=17.4,name=Apple TV" | ||
|
||
case .macOS_11, | ||
.macOS_12, | ||
case .macOS_12, | ||
.macOS_13, | ||
.macOS_14: | ||
return "platform=OS X" | ||
|
||
case .watchOS_6: | ||
return "OS=6.2.1,name=Apple Watch Series 4 - 44mm" | ||
case .watchOS_7: | ||
return "OS=7.2,name=Apple Watch Series 6 - 44mm" | ||
case .watchOS_8: | ||
return "OS=8.5,name=Apple Watch Series 6 - 44mm" | ||
case .watchOS_9: | ||
return "OS=9.4,name=Apple Watch Series 6 - 44mm" | ||
return "OS=9.4,name=Apple Watch Series 6 (44mm)" | ||
case .watchOS_10: | ||
return "OS=10.4,name=Apple Watch Series 6 - 44mm" | ||
return "OS=10.4,name=Apple Watch Series 9 (45mm)" | ||
} | ||
} | ||
|
||
var sdk: String { | ||
switch self { | ||
case .iOS_13, | ||
.iOS_14, | ||
.iOS_15, | ||
case .iOS_15, | ||
.iOS_16, | ||
.iOS_17: | ||
return "iphonesimulator" | ||
|
||
case .tvOS_13, | ||
.tvOS_14, | ||
.tvOS_15, | ||
case .tvOS_15, | ||
.tvOS_16, | ||
.tvOS_17: | ||
return "appletvsimulator" | ||
|
||
case .macOS_11: | ||
return "macosx11.1" | ||
case .macOS_12: | ||
return "macosx12.3" | ||
case .macOS_13: | ||
return "macosx13.3" | ||
case .macOS_14: | ||
return "macosx14.0" | ||
return "macosx14.4" | ||
|
||
case .watchOS_6, | ||
.watchOS_7, | ||
.watchOS_8, | ||
case .watchOS_8, | ||
.watchOS_9, | ||
.watchOS_10: | ||
return "watchsimulator" | ||
} | ||
} | ||
|
||
var shouldTest: Bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well this is nice at least! always |
||
switch self { | ||
case .iOS_13, | ||
.iOS_14, | ||
.iOS_15, | ||
.iOS_16, | ||
.iOS_17, | ||
.tvOS_13, | ||
.tvOS_14, | ||
.tvOS_15, | ||
.tvOS_16, | ||
.tvOS_17, | ||
.macOS_11, | ||
.macOS_12, | ||
.macOS_13, | ||
.macOS_14, | ||
.watchOS_8, | ||
.watchOS_9, | ||
.watchOS_10: | ||
return true | ||
|
||
case .watchOS_6, | ||
.watchOS_7: | ||
// watchOS did not support unit testing prior to Xcode 12.5. | ||
return false | ||
} | ||
} | ||
|
||
/// Whether the platform's Xcode version requires modern SPM integration in xcodebuild, given the removal of generate-xcodeproj. | ||
var requiresModernSPMIntegration: Bool { | ||
switch self { | ||
|
@@ -163,29 +107,22 @@ enum Platform: String, CustomStringConvertible { | |
|
||
var scheme: String { | ||
switch self { | ||
case .iOS_13, | ||
.iOS_14, | ||
.iOS_15, | ||
case .iOS_15, | ||
.iOS_16, | ||
.iOS_17: | ||
return "Valet iOS" | ||
|
||
case .tvOS_13, | ||
.tvOS_14, | ||
.tvOS_15, | ||
case .tvOS_15, | ||
.tvOS_16, | ||
.tvOS_17: | ||
return "Valet tvOS" | ||
|
||
case .macOS_11, | ||
.macOS_12, | ||
case .macOS_12, | ||
.macOS_13, | ||
.macOS_14: | ||
return "Valet Mac" | ||
|
||
case .watchOS_6, | ||
.watchOS_7, | ||
.watchOS_8, | ||
case .watchOS_8, | ||
.watchOS_9, | ||
.watchOS_10: | ||
return "Valet watchOS" | ||
|
@@ -271,27 +208,27 @@ enum Task: String, CustomStringConvertible { | |
// Our Package isn't set up with unit test targets, because SPM can't run unit tests in a codesigned environment. | ||
return false | ||
case .xcode: | ||
return platform.shouldTest | ||
return true | ||
} | ||
} | ||
} | ||
|
||
guard CommandLine.arguments.count > 2 else { | ||
print("Usage: build.swift platforms [spm|xcode]") | ||
exit(0) | ||
throw TaskError.code(1) | ||
Comment on lines
-281
to
+218
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From #300. Reverting so we can catch errors. |
||
} | ||
let rawPlatforms = CommandLine.arguments[1].components(separatedBy: ",") | ||
let rawTask = CommandLine.arguments[2] | ||
|
||
guard let task = Task(rawValue: rawTask) else { | ||
print("Received unknown task \(rawTask)") | ||
exit(0) | ||
throw TaskError.code(1) | ||
} | ||
|
||
let platforms = rawPlatforms.map { rawPlatform -> Platform in | ||
let platforms = try rawPlatforms.map { rawPlatform -> Platform in | ||
guard let platform = Platform(rawValue: rawPlatform) else { | ||
print("Received unknown platform type \(rawPlatform)") | ||
exit(0) | ||
throw TaskError.code(1) | ||
} | ||
|
||
return platform | ||
|
@@ -315,7 +252,7 @@ for platform in platforms { | |
deletedXcodeproj = true | ||
} catch { | ||
print("Could not delete Valet.xcodeproj due to error: \(error)") | ||
exit(0) | ||
throw TaskError.code(1) | ||
} | ||
} | ||
|
||
|
@@ -349,11 +286,7 @@ for platform in platforms { | |
xcodeBuildArguments.append("test") | ||
} | ||
|
||
do { | ||
try execute(commandPath: "/usr/bin/xcodebuild", arguments: xcodeBuildArguments) | ||
} catch { | ||
print("xcodebuild failed with error: \(error)") | ||
} | ||
try execute(commandPath: "/usr/bin/xcodebuild", arguments: xcodeBuildArguments) | ||
Comment on lines
-352
to
+289
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From #300. This was the big bad. |
||
|
||
if deletedXcodeproj { | ||
do { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well I broke Xcode 12 in addition to Xcode 11 builds in #308. Not ideal, but for the same reason we let #308 through, we're okay letting this change through.
Latest supported Xcode is 14. It'll be 15 on Friday.