-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Use StaticString for file
parameters in the XCTest overlay
#888
Conversation
Instead of just `String`. This makes it consistent with `fatalError`, `assert`, etc., as well as `swift-corelibs-xctest`
Looking forward to feedback on this! @gribozavr @mike-ferris-apple @modocache |
I think this is a good change. |
Use StaticString for `file` parameters in the XCTest overlay
Thanks! Happy to see this go through. |
This broke one of our internal tests; the offending code was something like this:
I can't see us being the only ones who would do this, so I think we need to revert this. If we want to avoid string construction, we could consider making the message parameters be autoclosures instead. |
@jrose-apple Hmm that's weird - this change contains no changes to the type of the |
Oops, I'm sorry. I completely misunderstood the problem and oversimplified the example.
So never mind, this was accounted for in the original request. |
The tests that broke are possibly because I was already working around this: https://github.com/apple/swift-package-manager/blob/master/Tests/Functional/Utilities.swift#L18-L22 #if os(Linux)
typealias MyString = StaticString
#else
typealias MyString = String
#endif |
This broke SwiftLint's test. realm/SwiftLint#425 |
Originally brought up in Quick#242, `XCTest` methods now take `StaticString` (see swiftlang/swift#888 for details), so this won't compile just yet.
Originally brought up in Quick#242, `XCTest` methods now take `StaticString` (see swiftlang/swift#888 for details). Unfortunately this won't compile now because `StaticString` is not available in Objective-C!
This is also posing a challenge for the Nimble test matcher framework (see here). We expose both Swift and Objective-C APIs which funnel into Conceptually, the |
Resolve conflicts with master
The
XCTest
SDK overlay contains a set ofXCTAssert*
functions, each of which hasfile
andline
parameters, just asassert
,fatalError
, etc. do, however theXCAssert
functions useString
as the type of theirfile
parameter, instead ofStaticString
as the other functions in the stdlib do for this purpose. This PR would change these parameters to be typed asStaticString
.See related discussion on SR-308 and swiftlang/swift-corelibs-xctest#27 where @gribozavr suggested this change.
Advantages
XCTest
overlay functions in line with how they are defined inswift-corelibs-xctest
StaticString
Disadvantages
XCTAssert* function. Such functions would have a
fileparameter themselves which would also need to be updated to use
StaticString`.