Add @autoclosure to message parameter, so it's constructed only if it's needed.

[kovpas]

Sometimes it's nice to have a complex "conditional" XCTAssert message.

XCTAssertEqual(a, b, "\(String(idx, radix: 16)) \(a ? "exists in" : "does not exist in") mutable and \(b ? "exists in" : "does not exist in") immutable character set")

This is already a small performance impact, but it's nothing too bad, unless it's in a loop. There was a test in NSCharacterSet, which used to take quite a lot of time: https://github.com/apple/swift-corelibs-foundation/pull/149/files

I think it would be nice if message param is only evaluated if it's needed.

The test, that used to take 116 sec on my linux machine, takes only 3.5 seconds with this change.

[modocache]

The XCTAssert() functions in Objective-C XCTest do not wrap their message parameters in @autoclosure. As a result, I think this request may conflict with this project's mission statement:

This version of XCTest uses the same API as the XCTest you are familiar with from Xcode. Our goal is to enable your project's tests to run on all Swift platforms without having to rewrite them.

Here's the Swift XCTAssert() signature from Objective-C XCTest:

public func XCTAssert(@autoclosure expression: () -> BooleanType, _ message: String = default, file: String = default, line: UInt = default)

[kovpas]

Oh, I see. This is an API change indeed. However, it doesn't lead to any changes to tests.

[phausler]

There are already a few differentials and I think this might be something that would perhaps be an improvement to both; in that the messages could have side-effects that would not be intended unless in a failure case. e.g. calculating a debug description etc.

I would be interested to see how this change would work if it were in the XCTest overlay as well performance wise; faster tests usually mean people are more willing to have more tests execute for the one thing they are iterating on.

[kovpas]

@modocache current implementation already have slightly different signature:

public func XCTAssert(@autoclosure expression: () -> BooleanType, _ message: String = "", file: StaticString = __FILE__, line: UInt = __LINE__)

[modocache]

Hey yeah, look at that! What the heck is a StaticString?

[kovpas]

How about "", __FILE__ and __LINE__?

What's the final goal? To make sure it's letter-by-letter the same as the previous declaration? It's already not. If the goal is

to enable your project's tests to run on all Swift platforms without having to rewrite them.

Then @autoclosure in message param doesn't go against that.

[phausler]

@modocache StaticString is specifically anything written as "" instead of a variable. This was initially required for the linux implementation to even work (at least at the time of when I originally wrote that portion). It may no longer be needed

[gribozavr]

@phausler No, it was added to the library for a different reason. It was added to represent a string that does not need to be dynamically allocated (like a file name in an assertion). It is cheaper than String.

[modocache]

@kovpas I agree! If we're willing to deviate slightly from the function signatures in Objective-C XCTest, using an @autoclosure for the message parameter seems like a great idea! Might even be worth filing a radar for Objective-C XCTest 😉

How about "", __FILE__ and __LINE__?

I think these are the default parameters for Objective-C XCTest as well, they're just represented as default in the bridged Swift headers.

[eschaton]

This is correct, "", FILE, and LINE are the default values in the Xcode XCTest assertions. They're implemented in the XCTest overlay in the Swift codebase, so you can see them there.

On Dec 19, 2015, at 10:15 AM, Brian Gesiak [email protected] wrote:

@kovpas I agree! If we're willing to deviate slightly from the function signatures in Objective-C XCTest, using an @autoclosure for the message parameter seems like a great idea! Might even be worth filing a radar for Objective-C XCTest

How about "", FILE and LINE?

I think these are the default parameters for Objective-C XCTest as well, they're just represented as default in the bridged Swift headers.

―
Reply to this email directly or view it on GitHub.

[mike-ferris]

I like this change. Could you make a similar pull request for the XCTest overlay in the swift project as well?

[kovpas]

@mike-ferris-apple sure, will do today.

[mike-ferris]

Also, some of the other pull requests we've taken lately have caused this to have conflicts. Do you mind rebasing it?

[kovpas]

@mike-ferris-apple done.

[kovpas]

@mike-ferris-apple also created a pull request in the swift project: swiftlang/swift#913

[mike-ferris]

Thanks!