metova / MetovaTestKit

A collection of useful test helpers designed to ease the burden of writing tests for iOS applications.
MIT License
23 stars 2 forks source link

Deprecate "MTK" in favor of "MetovaTestKit" #26

Closed lgauthier closed 6 years ago

lgauthier commented 7 years ago

The name of this repository is "MetovaTestKit", the name of the CocoaPod is "MTK", and the README references the names "Metova Test Kit" and "MTK". This leads to a bit of confusion. Furthermore, "MTK" isn't very descriptive. It's analogous to a bad variable name.

In light of this, I'd like to deprecate "MTK" in favor of "MetovaTestKit". I don't think it's necessary for us to send a PR to CocoaPods' master spec repo—and we wouldn't want to since that would break existing Podfiles that reference MTK. Instead, I believe we can simply rename our podspec, push the new podspec, then run:

pod trunk deprecate MTK --in-favor-of=MetovaTestKit

However, I don't want to just rename MTK. I'd like to make a number of breaking changes to make the framework look and feel like a Swift framework rather than an Objective-C framework—after all, it's written in Swift. So, I think we should drop the "MTK" prefix from all assertions, and name them with lowerCamelCase as per the Swift API Design Guidelines.

I believe we originally went with the all caps "MTK" prefix in order to mirror the naming conventions of the XCTest assertions, but I don't think there's really any value in doing that. After all, all the new methods that have been added to XCTest no longer follow that convention (e.g. expectation(description:)).

The practice of prefixing class/method names with initials came from Objective-C where it was necessary in order to avoid naming conflicts. However, with Swift this isn't a problem. Even if a project was importing MetovaTestKit along with another framework that also happened to have an extension on XCTestCase with a method with the same name as one from MetovaTestKit (e.g. assertControl(_:sends:to:for:)), the naming conflict can be resolved by fully qualifying it with the name of the module:

MetovaTestKit.assertControl(testVC.myControl, sends: #selector(MyViewController.didTapControl(_:)), to: testVC, for: .touchUpInside)

Any thoughts or concerns?

schrismartin commented 7 years ago

Perhaps I'm bikeshedding a little bit here, but I like the idea of maintaining the parity in prefix between MTK and XCT. If MTK made the direct use of XCT obsolete (in a way that Nimble does, for example), I'd be all for stripping the prefix.

Internally, Metova still uses XTC, and I like having the consistency of the prefixes (or lack thereof) for assertions.

I'm all for the module rename, however.

nhgrif commented 7 years ago

So, originally, I had put the MTK prefix less as an Objective-C thing and more as an alignment to XCT (which, I know, is done that way because Objective-C...).

We could go ahead and wrap all of the XCT stuff, thereby making XCT usage obsolete if you pull in MTK (even if it matched the same pattern).

But I was actually somewhat originally concerned about conflicting test libraries, but as @lgauthier points out, Swift has a solution for that. And furthermore, any file that used MTK would have to add import MetovaTestKit explicitly to the top of the file (which is different from Objective-C and the way importing a header also then imports all the things that header imports, for example).

lgauthier commented 7 years ago

I'm actually really surprised that Apple hasn't provided Swift-specific names for the XCTest assertions. It seems like everything else in the SDK now has custom Swift names that conform to the Swift Style Guide. I'm curious as to whether there's any reason for that.

nhgrif commented 7 years ago

Well, in almost every other case, we're talking about a class, struct, or typealias, right? How many other cases are we talking about free functions?

lgauthier commented 7 years ago

@nhgrif What exactly are you referring to?

nhgrif commented 7 years ago

When you change NSDate to Date, you're changing the name of a class. When you change NSTimeInterval to TimeInterval, you're changing the name of typealias.

All of the things with the XCT prefix are free functions. They're not types.

https://stackoverflow.com/a/4863328/2792531

lgauthier commented 7 years ago

Ah, you're referring to all the Swift renamings in general. Can global C functions not be given a Swift specific name using NS_SWIFT_NAME? I'm assuming Apple could annotate the XCTest assertions with NS_SWIFT_NAME to expose a function name to Swift that adheres to the Swift API Design Guidelines. Something sort of like this:

void XCTAssertTrue(BOOL) NS_SWIFT_NAME(assertTrue(_:));

If that's possible, then I'm curious as to why Apple hasn't done that. If there's a reason why they haven't done that, then I'm wondering if that reason would also apply to why we shouldn't rename the MetovaTestKit assertions.

lgauthier commented 6 years ago

PR #29 addresses the renaming of MTK to MetovaTestKit.