Skip to content

[SR-5643] Adopt Swift 4 API adjustments from Apple XCTest #198

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

Merged
merged 4 commits into from
Aug 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions Sources/XCTest/Private/PerformanceMeter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,22 @@ internal protocol PerformanceMeterDelegate {
/// as average, and standard deviation
/// - Parameter file: The source file name where the measurement was invoked
/// - Parameter line: The source line number where the measurement was invoked
func recordMeasurements(results: String, file: StaticString, line: UInt)
func recordMeasurements(results: String, file: StaticString, line: Int)

/// Reports a test failure from the analysis of performance measurements.
/// This can currently be caused by an unexpectedly large standard deviation
/// calculated over the data.
/// - Parameter description: An explanation of the failure
/// - Parameter file: The source file name where the measurement was invoked
/// - Parameter line: The source line number where the measurement was invoked
func recordFailure(description: String, file: StaticString, line: UInt)
func recordFailure(description: String, file: StaticString, line: Int)

/// Reports a misuse of the `PerformanceMeter` API, such as calling `
/// startMeasuring` multiple times.
/// - Parameter description: An explanation of the misuse
/// - Parameter file: The source file name where the misuse occurred
/// - Parameter line: The source line number where the misuse occurred
func recordAPIViolation(description: String, file: StaticString, line: UInt)
func recordAPIViolation(description: String, file: StaticString, line: Int)
}

/// - Bug: This class is intended to be `internal` but is public to work around
Expand Down Expand Up @@ -97,16 +97,16 @@ public final class PerformanceMeter {
private let metrics: [PerformanceMetric]
private let delegate: PerformanceMeterDelegate
private let invocationFile: StaticString
private let invocationLine: UInt
private let invocationLine: Int

private init(metrics: [PerformanceMetric], delegate: PerformanceMeterDelegate, file: StaticString, line: UInt) {
private init(metrics: [PerformanceMetric], delegate: PerformanceMeterDelegate, file: StaticString, line: Int) {
self.metrics = metrics
self.delegate = delegate
self.invocationFile = file
self.invocationLine = line
}

static func measureMetrics(_ metricNames: [String], delegate: PerformanceMeterDelegate, file: StaticString = #file, line: UInt = #line, for block: (PerformanceMeter) -> Void) {
static func measureMetrics(_ metricNames: [String], delegate: PerformanceMeterDelegate, file: StaticString = #file, line: Int = #line, for block: (PerformanceMeter) -> Void) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make metricNames: [PerformanceMetric] too? As they're strings, we have to map from PerformanceMetric to String down below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm..

One of the design goals with the PerformanceMeter and WallClockTimeMetric classes was for them to be entirely decoupled from the XCTest-specific stuff. i.e. they are conceptually a lower layer upon which the XCTest API sits. Ideally they would exist in their own separate module, but it's not currently possible to do that while still keeping XCTest library product and module standalone. See #109 for earlier discussion.

How do you feel about this given that context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. A possible (future) improvement could be to create a similar enum at the PerformanceMeter layer.

do {
let metrics = try self.metrics(forNames: metricNames)
let meter = PerformanceMeter(metrics: metrics, delegate: delegate, file: file, line: line)
Expand All @@ -116,15 +116,15 @@ public final class PerformanceMeter {
}
}

func startMeasuring(file: StaticString = #file, line: UInt = #line) {
func startMeasuring(file: StaticString = #file, line: Int = #line) {
guard state == .iterationUnstarted else {
return recordAPIViolation(.startMeasuringAlreadyCalled, file: file, line: line)
}
state = .iterationStarted
metrics.forEach { $0.startMeasuring() }
}

func stopMeasuring(file: StaticString = #file, line: UInt = #line) {
func stopMeasuring(file: StaticString = #file, line: Int = #line) {
guard state != .iterationUnstarted else {
return recordAPIViolation(.stopBeforeStarting, file: file, line: line)
}
Expand Down Expand Up @@ -195,7 +195,7 @@ public final class PerformanceMeter {
}
}

private func recordAPIViolation(_ error: Error, file: StaticString, line: UInt) {
private func recordAPIViolation(_ error: Error, file: StaticString, line: Int) {
state = .measurementAborted
delegate.recordAPIViolation(description: String(describing: error), file: file, line: line)
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/XCTest/Private/PrintObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class PrintObserver: XCTestObservation {
printAndFlush("Test Case '\(testCase.name)' started at \(dateFormatter.string(from: testCase.testRun!.startDate!))")
}

func testCase(_ testCase: XCTestCase, didFailWithDescription description: String, inFile filePath: String?, atLine lineNumber: UInt) {
func testCase(_ testCase: XCTestCase, didFailWithDescription description: String, inFile filePath: String?, atLine lineNumber: Int) {
let file = filePath ?? "<unknown>"
printAndFlush("\(file):\(lineNumber): error: \(testCase.name) : \(description)")
}
Expand Down Expand Up @@ -68,7 +68,7 @@ internal class PrintObserver: XCTestObservation {
}

extension PrintObserver: XCTestInternalObservation {
func testCase(_ testCase: XCTestCase, didMeasurePerformanceResults results: String, file: StaticString, line: UInt) {
func testCase(_ testCase: XCTestCase, didMeasurePerformanceResults results: String, file: StaticString, line: Int) {
printAndFlush("\(file):\(line): Test Case '\(testCase.name)' measured \(results)")
}
}
2 changes: 1 addition & 1 deletion Sources/XCTest/Private/XCPredicateExpectation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal class XCPredicateExpectation: XCTestExpectation {
internal let handler: XCPredicateExpectationHandler?
private let evaluationInterval = 0.01

internal init(predicate: NSPredicate, object: AnyObject, description: String, file: StaticString, line: UInt, testCase: XCTestCase, handler: XCPredicateExpectationHandler? = nil) {
internal init(predicate: NSPredicate, object: AnyObject, description: String, file: StaticString, line: Int, testCase: XCTestCase, handler: XCPredicateExpectationHandler? = nil) {
self.predicate = predicate
self.object = object
self.handler = handler
Expand Down
4 changes: 2 additions & 2 deletions Sources/XCTest/Private/XCTestInternalObservation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ internal protocol XCTestInternalObservation: XCTestObservation {
/// reported, if available.
/// - Parameter line: The line number in the source file where the failure
/// was reported.
func testCase(_ testCase: XCTestCase, didMeasurePerformanceResults results: String, file: StaticString, line: UInt)
func testCase(_ testCase: XCTestCase, didMeasurePerformanceResults results: String, file: StaticString, line: Int)
}

// All `XCInternalTestObservation` methods are optional, so empty default implementations are provided
internal extension XCTestInternalObservation {
func testCase(_ testCase: XCTestCase, didMeasurePerformanceResults results: String, file: StaticString, line: UInt) {}
func testCase(_ testCase: XCTestCase, didMeasurePerformanceResults results: String, file: StaticString, line: Int) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public extension XCTestCase {
/// between these environments. To ensure compatibility of tests between
/// swift-corelibs-xctest and Apple XCTest, it is not recommended to pass
/// explicit values for `file` and `line`.
@discardableResult func expectation(description: String, file: StaticString = #file, line: UInt = #line) -> XCTestExpectation {
@discardableResult func expectation(description: String, file: StaticString = #file, line: Int = #line) -> XCTestExpectation {
let expectation = XCTestExpectation(
description: description,
file: file,
Expand Down Expand Up @@ -68,7 +68,7 @@ public extension XCTestCase {
/// these environments. To ensure compatibility of tests between
/// swift-corelibs-xctest and Apple XCTest, it is not recommended to pass
/// explicit values for `file` and `line`.
func waitForExpectations(timeout: TimeInterval, file: StaticString = #file, line: UInt = #line, handler: XCWaitCompletionHandler? = nil) {
func waitForExpectations(timeout: TimeInterval, file: StaticString = #file, line: Int = #line, handler: XCWaitCompletionHandler? = nil) {
// Mirror Objective-C XCTest behavior; display an unexpected test
// failure when users wait without having first set expectations.
// FIXME: Objective-C XCTest raises an exception for most "API
Expand Down Expand Up @@ -139,7 +139,7 @@ public extension XCTestCase {
// If the test failed, send an error object.
error = NSError(
domain: XCTestErrorDomain,
code: XCTestErrorCode.timeoutWhileWaiting.rawValue,
code: XCTestError.Code.timeoutWhileWaiting.rawValue,
userInfo: [:])
}
completionHandler(error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public extension XCTestCase {
/// first successful evaluation will fulfill the expectation. If provided,
/// the handler can override that behavior which leaves the caller
/// responsible for fulfilling the expectation.
@discardableResult func expectation(for predicate: NSPredicate, evaluatedWith object: AnyObject, file: StaticString = #file, line: UInt = #line, handler: XCPredicateExpectationHandler? = nil) -> XCTestExpectation {
@discardableResult func expectation(for predicate: NSPredicate, evaluatedWith object: AnyObject, file: StaticString = #file, line: Int = #line, handler: XCPredicateExpectationHandler? = nil) -> XCTestExpectation {
let expectation = XCPredicateExpectation(
predicate: predicate,
object: object,
Expand Down
6 changes: 3 additions & 3 deletions Sources/XCTest/Public/Asynchronous/XCTestExpectation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
public class XCTestExpectation {
internal let description: String
internal let file: StaticString
internal let line: UInt
internal let line: Int

internal var isFulfilled = false
internal weak var testCase: XCTestCase?

internal init(description: String, file: StaticString, line: UInt, testCase: XCTestCase) {
internal init(description: String, file: StaticString, line: Int, testCase: XCTestCase) {
self.description = description
self.file = file
self.line = line
Expand All @@ -47,7 +47,7 @@ public class XCTestExpectation {
/// between these environments. To ensure compatibility of tests between
/// swift-corelibs-xctest and Apple XCTest, it is not recommended to pass
/// explicit values for `file` and `line`.
public func fulfill(_ file: StaticString = #file, line: UInt = #line) {
public func fulfill(_ file: StaticString = #file, line: Int = #line) {
// FIXME: Objective-C XCTest emits failures when expectations are
// fulfilled after the test cases that generated those
// expectations have completed. Similarly, this should cause an
Expand Down
2 changes: 1 addition & 1 deletion Sources/XCTest/Public/XCAbstractTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ open class XCTest {
}

/// Number of test cases. Must be overridden by subclasses.
open var testCaseCount: UInt {
open var testCaseCount: Int {
fatalError("Must be overridden by subclasses.")
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/XCTest/Public/XCTAssert.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private func _XCTEvaluateAssertion(_ assertion: _XCTAssertion, message: @autoclo
currentTestCase.recordFailure(
withDescription: "\(result.failureDescription(assertion)) - \(message())",
inFile: String(describing: file),
atLine: line,
atLine: Int(line),
expected: result.isExpected)
}
}
Expand Down
54 changes: 38 additions & 16 deletions Sources/XCTest/Public/XCTestCase+Performance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,30 @@
// Methods on XCTestCase for testing the performance of code blocks.
//

/// Records wall clock time in seconds between `startMeasuring`/`stopMeasuring`.
public let XCTPerformanceMetric_WallClockTime = WallClockTimeMetric.name
public struct XCTPerformanceMetric : RawRepresentable, Equatable, Hashable {
public private(set) var rawValue: String

public init(_ rawValue: String) {
self.rawValue = rawValue
}

public init(rawValue: String) {
self.rawValue = rawValue
}

public var hashValue: Int {
return rawValue.hashValue
}

public static func ==(_ lhs: XCTPerformanceMetric, _ rhs: XCTPerformanceMetric) -> Bool {
return lhs.rawValue == rhs.rawValue
}
}

public extension XCTPerformanceMetric {
/// Records wall clock time in seconds between `startMeasuring`/`stopMeasuring`.
public static let wallClockTime = XCTPerformanceMetric(rawValue: WallClockTimeMetric.name)
}

/// The following methods are called from within a test method to carry out
/// performance testing on blocks of code.
Expand All @@ -21,11 +43,11 @@ public extension XCTestCase {
/// The names of the performance metrics to measure when invoking `measure(block:)`.
/// Returns `XCTPerformanceMetric_WallClockTime` by default. Subclasses can
/// override this to change the behavior of `measure(block:)`
class func defaultPerformanceMetrics() -> [String] {
return [XCTPerformanceMetric_WallClockTime]
class var defaultPerformanceMetrics: [XCTPerformanceMetric] {
return [.wallClockTime]
}

/// Call from a test method to measure resources (`defaultPerformanceMetrics()`)
/// Call from a test method to measure resources (`defaultPerformanceMetrics`)
/// used by the block in the current process.
///
/// func testPerformanceOfMyFunction() {
Expand All @@ -49,8 +71,8 @@ public extension XCTestCase {
/// these methods are not exactly identical between these environments. To
/// ensure compatibility of tests between swift-corelibs-xctest and Apple
/// XCTest, it is not recommended to pass explicit values for `file` and `line`.
func measure(file: StaticString = #file, line: UInt = #line, block: () -> Void) {
measureMetrics(type(of: self).defaultPerformanceMetrics(),
func measure(file: StaticString = #file, line: Int = #line, block: () -> Void) {
measureMetrics(type(of: self).defaultPerformanceMetrics,
automaticallyStartMeasuring: true,
file: file,
line: line,
Expand All @@ -66,7 +88,7 @@ public extension XCTestCase {
/// may interfere the API will measure them separately.
///
/// func testMyFunction2_WallClockTime() {
/// measureMetrics(type(of: self).defaultPerformanceMetrics(), automaticallyStartMeasuring: false) {
/// measureMetrics(type(of: self).defaultPerformanceMetrics, automaticallyStartMeasuring: false) {
///
/// // Do setup work that needs to be done for every iteration but
/// // you don't want to measure before the call to `startMeasuring()`
Expand Down Expand Up @@ -103,12 +125,12 @@ public extension XCTestCase {
/// these methods are not exactly identical between these environments. To
/// ensure compatibility of tests between swift-corelibs-xctest and Apple
/// XCTest, it is not recommended to pass explicit values for `file` and `line`.
func measureMetrics(_ metrics: [String], automaticallyStartMeasuring: Bool, file: StaticString = #file, line: UInt = #line, for block: () -> Void) {
func measureMetrics(_ metrics: [XCTPerformanceMetric], automaticallyStartMeasuring: Bool, file: StaticString = #file, line: Int = #line, for block: () -> Void) {
guard _performanceMeter == nil else {
return recordAPIViolation(description: "Can only record one set of metrics per test method.", file: file, line: line)
}

PerformanceMeter.measureMetrics(metrics, delegate: self, file: file, line: line) { meter in
PerformanceMeter.measureMetrics(metrics.map({ $0.rawValue }), delegate: self, file: file, line: line) { meter in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could get rid of this map if we move to [XCTPerformanceMetric] across the board.

self._performanceMeter = meter
if automaticallyStartMeasuring {
meter.startMeasuring(file: file, line: line)
Expand All @@ -125,7 +147,7 @@ public extension XCTestCase {
/// these methods are not exactly identical between these environments. To
/// ensure compatibility of tests between swift-corelibs-xctest and Apple
/// XCTest, it is not recommended to pass explicit values for `file` and `line`.
func startMeasuring(file: StaticString = #file, line: UInt = #line) {
func startMeasuring(file: StaticString = #file, line: Int = #line) {
guard let performanceMeter = _performanceMeter, !performanceMeter.didFinishMeasuring else {
return recordAPIViolation(description: "Cannot start measuring. startMeasuring() is only supported from a block passed to measureMetrics(...).", file: file, line: line)
}
Expand All @@ -140,7 +162,7 @@ public extension XCTestCase {
/// these methods are not exactly identical between these environments. To
/// ensure compatibility of tests between swift-corelibs-xctest and Apple
/// XCTest, it is not recommended to pass explicit values for `file` and `line`.
func stopMeasuring(file: StaticString = #file, line: UInt = #line) {
func stopMeasuring(file: StaticString = #file, line: Int = #line) {
guard let performanceMeter = _performanceMeter, !performanceMeter.didFinishMeasuring else {
return recordAPIViolation(description: "Cannot stop measuring. stopMeasuring() is only supported from a block passed to measureMetrics(...).", file: file, line: line)
}
Expand All @@ -149,18 +171,18 @@ public extension XCTestCase {
}

extension XCTestCase: PerformanceMeterDelegate {
internal func recordAPIViolation(description: String, file: StaticString, line: UInt) {
internal func recordAPIViolation(description: String, file: StaticString, line: Int) {
recordFailure(withDescription: "API violation - \(description)",
inFile: String(describing: file),
atLine: line,
expected: false)
}

internal func recordMeasurements(results: String, file: StaticString, line: UInt) {
XCTestObservationCenter.shared().testCase(self, didMeasurePerformanceResults: results, file: file, line: line)
internal func recordMeasurements(results: String, file: StaticString, line: Int) {
XCTestObservationCenter.shared.testCase(self, didMeasurePerformanceResults: results, file: file, line: line)
}

internal func recordFailure(description: String, file: StaticString, line: UInt) {
internal func recordFailure(description: String, file: StaticString, line: Int) {
recordFailure(withDescription: "failed: " + description, inFile: String(describing: file), atLine: line, expected: true)
}
}
4 changes: 2 additions & 2 deletions Sources/XCTest/Public/XCTestCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ open class XCTestCase: XCTest {
/// A private setter for the name of this test case.
private var _name: String

open override var testCaseCount: UInt {
open override var testCaseCount: Int {
return 1
}

Expand Down Expand Up @@ -99,7 +99,7 @@ open class XCTestCase: XCTest {
/// - Parameter expected: `true` if the failure being reported was the
/// result of a failed assertion, `false` if it was the result of an
/// uncaught exception.
open func recordFailure(withDescription description: String, inFile filePath: String, atLine lineNumber: UInt, expected: Bool) {
open func recordFailure(withDescription description: String, inFile filePath: String, atLine lineNumber: Int, expected: Bool) {
testRun?.recordFailure(
withDescription: description,
inFile: filePath,
Expand Down
8 changes: 4 additions & 4 deletions Sources/XCTest/Public/XCTestCaseRun.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,21 @@
open class XCTestCaseRun: XCTestRun {
open override func start() {
super.start()
XCTestObservationCenter.shared().testCaseWillStart(testCase)
XCTestObservationCenter.shared.testCaseWillStart(testCase)
}

open override func stop() {
super.stop()
XCTestObservationCenter.shared().testCaseDidFinish(testCase)
XCTestObservationCenter.shared.testCaseDidFinish(testCase)
}

open override func recordFailure(withDescription description: String, inFile filePath: String?, atLine lineNumber: UInt, expected: Bool) {
open override func recordFailure(withDescription description: String, inFile filePath: String?, atLine lineNumber: Int, expected: Bool) {
super.recordFailure(
withDescription: "\(test.name) : \(description)",
inFile: filePath,
atLine: lineNumber,
expected: expected)
XCTestObservationCenter.shared().testCase(
XCTestObservationCenter.shared.testCase(
testCase,
didFailWithDescription: description,
inFile: filePath,
Expand Down
Loading