Additional Swift 3 API naming updates #88

briancroom · 2016-04-04T02:22:57Z

#82 got us started with updating our own APIs to match the Swift 3 guidelines, but there are a number of other changes required for us to be fully compatible. Here, I've adjusted a few public methods to match how they are imported into Swift from Apple XCTest with the Swift 3 toolchain. I have also made a handful of changes to internal APIs to bring them in line with the Swift 3 naming guidelines. See the commit messages for more specifics!

gribozavr · 2016-04-04T02:25:37Z

Sources/XCTest/XCTestCase.swift

@@ -189,7 +189,7 @@ 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`.
-    public func expectationWithDescription(description: String, file: StaticString = #file, line: UInt = #line) -> XCTestExpectation {
+    public func expectation(withDescription description: String, file: StaticString = #file, line: UInt = #line) -> XCTestExpectation {


"With" is usually a vacuous preposition that does not carry any semantics. I think this is one of those cases. (Other cases in XCTest may also be like that.)

Agreed. However this is the method name as generated by the importer with the March 24 toolchain from XCTest.framework. Do you expect additional changes to the importer which would automatically remove "with"? Or is this a situation where the Objective-C header should be annotated with the swift_name attribute?

(Interestingly, it appears that https://github.com/apple/swift-3-api-guidelines-review doesn't include XCTest.framework)

This should match exactly what the importer does, which is leaving the with in place (and this is very much an intentional choice).

@parkera is there any additional discussion around this particular choice you could point me to? I'm curious about the context on this.

It was a discussion we had in our Swift 3 naming 'working group'.

The summary is that we felt that the importer could not automatically make the right decision about if a 'with' was important or not in an API. It is used to mean different things in different APIs. Therefore we decided to stick with a default of leaving it in place for imported API.

We can of course change our minds about the names, but then we're talking about changing it for all clients on all platforms. Since this version of XCTest is attempting to keep the same API, and there isn't a plan to change the Darwin version of this method, we should keep the name the same for now.

Thanks, that's exactly the info I was looking for.

modocache · 2016-04-04T14:17:20Z

Looks good to me! @parkera or @mike-ferris-apple, could you kick off the tests?

Also, I'm curious: what's the best way to see the Swift headers generated for Apple XCTest? I use "Jump to Definition" in Xcode (with my desired toolchain selected) to see the Swift headers. Is there a better way, perhaps something I can invoke from the command line?

mike-ferris · 2016-04-04T14:37:00Z

@swift-ci please test

modocache · 2016-04-04T14:51:19Z

Ruh-roh, build failure. @briancroom, looks like you missed a spot:

Sources/XCTest/XCTestCase.swift:265:21: error: value of type 'XCTestExpectation' has no member 'fulfilled'
                if !expectation.fulfilled {
                    ^~~~~~~~~~~ ~~~~~~~~~

parkera · 2016-04-04T15:01:00Z

One trick I've been using is to start up the swift REPL after building the whole stack from scratch. You can tab-complete in there to get a list of API on a type.

modocache · 2016-04-04T15:49:53Z

utils/swift-api-dump.py seems like what I need, although it doesn't cover XCTest--see SR-1153. Maybe I'll send a pull request... 😉

parkera · 2016-04-04T15:58:32Z

Nice, I didn't know about that script. Thanks @modocache.

briancroom · 2016-04-04T17:13:01Z

Oh man, thanks CI for pointing out my incomplete commit 😳 I've fixed that now, rebased, and added one additional Swift 3 import change I had previously missed - default nil values for optional handler params.

Also thanks for the swift-api-dump.py tip - so far I've been viewing the Generated Interface from within Xcode to see these things.

modocache · 2016-04-06T01:59:02Z

Ping @mike-ferris-apple for another test kickoff -- how about "@swift-ci please test and merge"? 😁

modocache · 2016-04-06T03:54:37Z

It turns out swift-api-dump.py can generate XCTest headers as well, you just need to know which arguments to pass. I uploaded the current set of headers here, along with the command: https://github.com/modocache/XCTest.swift

mike-ferris · 2016-04-06T14:33:30Z

@swift-ci please test

briancroom · 2016-04-06T16:40:14Z

Ah, looks like CI failed because #90 was merged which contains code that needs updating. Hang tight...

- Lowercased enum cases - Boolean properties prefixed with `is`

This is to match another Swift 3 API importer change.

briancroom · 2016-04-06T16:46:18Z

Rebased and fixed up now. @mike-ferris-apple could I bother you for another CI test request?

mike-ferris · 2016-04-06T18:03:30Z

@swift-ci please test

mike-ferris · 2016-04-06T18:04:04Z

Ah, oops. Didn't see Harlan's request there.

harlanhaskins · 2016-04-06T18:04:42Z

Nah, I'm not on the CI list quite yet 👍

modocache · 2016-04-06T19:27:09Z

One day swiftlang/swift#2036 will be merged and the Swift Test OS X Platform check will succeed. Today is not that day 😂

In the meantime, looks like Linux passed, so I'm merging this. Thanks as always, @briancroom! 💯

gribozavr reviewed Apr 4, 2016
View reviewed changes

briancroom force-pushed the swift-3-api-updates branch 2 times, most recently from 31afda4 to 4d70c0c Compare April 4, 2016 17:11

briancroom added 3 commits April 6, 2016 12:43

Update public API methods names to match how Swift 3 imports them

cb77946

Additional naming updates for Swift 3

f50234e

- Lowercased enum cases - Boolean properties prefixed with `is`

Add a nil default value for optional handler parameters

1ad6f7e

This is to match another Swift 3 API importer change.

briancroom force-pushed the swift-3-api-updates branch from 4d70c0c to 1ad6f7e Compare April 6, 2016 16:44

briancroom mentioned this pull request Apr 6, 2016

[SE-0046] Implemented consistent function labels #91

Closed

modocache merged commit f935137 into swiftlang:master Apr 6, 2016

modocache mentioned this pull request Apr 6, 2016

Use XCTestRun to execute tests and report results #86

Merged

briancroom deleted the swift-3-api-updates branch April 6, 2016 19:32

briancroom mentioned this pull request Apr 13, 2016

Added expectations to server request test cases Kitura/Kitura#349

Merged

2 tasks

swift-ci mentioned this pull request Apr 4, 2016

[SR-1063] Have XCTestObservationCenter conform to new Swift 3 naming rules #397

Closed

Additional Swift 3 API naming updates #88

Additional Swift 3 API naming updates #88

Uh oh!

Conversation

briancroom commented Apr 4, 2016

Uh oh!

gribozavr Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

briancroom Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

parkera Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

briancroom Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

parkera Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

briancroom Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

modocache commented Apr 4, 2016

Uh oh!

mike-ferris commented Apr 4, 2016

Uh oh!

modocache commented Apr 4, 2016

Uh oh!

parkera commented Apr 4, 2016

Uh oh!

modocache commented Apr 4, 2016

Uh oh!

parkera commented Apr 4, 2016

Uh oh!

briancroom commented Apr 4, 2016

Uh oh!

modocache commented Apr 6, 2016

Uh oh!

modocache commented Apr 6, 2016

Uh oh!

mike-ferris commented Apr 6, 2016

Uh oh!

briancroom commented Apr 6, 2016

Uh oh!

briancroom commented Apr 6, 2016

Uh oh!

mike-ferris commented Apr 6, 2016

Uh oh!

mike-ferris commented Apr 6, 2016

Uh oh!

harlanhaskins commented Apr 6, 2016

Uh oh!

modocache commented Apr 6, 2016

Uh oh!

Uh oh!