I’m a bit confused here… Since the result of breaching your subclassing contract is a compile-time error, how are you intending to test that using XCTest?
On the other hand, if you want to write a test that subclasses the class, and uses it as a Library consumer would, why would you use @testable? Wouldn’t you just import the module normally and verify the contract that way (and thus getting a compile failure if the open/public stuff is wrong). That’s not a Unit Test, that’s a functional test, so I’d probably make that a separate test target anyway. Scott > On Jul 27, 2016, at 4:41 PM, David Owens II <[email protected]> wrote: > > I brought this up in review, but there seems to be a serious testability > problem here because of how special @testable is. > > // This class is not subclassable outside of ModuleA. > public class NonSubclassableParentClass { > // This method is not overridable outside of ModuleA. > public func foo() {} > > // This method is not overridable outside of ModuleA because > // its class restricts its access level. > // It is not invalid to declare it as `open`. > open func bar() {} > > // The behavior of `final` methods remains unchanged. > public final func baz() {} > } > > In a unit test, I *can* subclass `NonSubclassableParentClass`, the access > level of `NonSubclassableParentClass` is irrelevant. There’s now no > programatic way to ensure that I’m actually testing the contract that I had > intended to provide to the consumers of my framework (that my class is > subclassable). Is this really the intention? > > The “fix”, on the authors end, is to create another target that consumes the > output framework to ensure my contract is actually what I wanted (and make > sure that it’s not a special test target!). No one is going to do this. > > Sure, maybe a code review might catch it. Or I can write a custom linter to > validate for this. Do we really want `open` members in non `open` types? The > issue with `public` members on `internal` types is much less concerning as > the `internal` type isn’t being exposed to others to begin with. > > -David > > >> On Jul 27, 2016, at 3:18 PM, Scott James Remnant via swift-evolution >> <[email protected] <mailto:[email protected]>> wrote: >> >> I realize that there’s no review needed, but I actually wanted to give a >> hearty 👏 to the authors and commenters of this proposal, because I genuinely >> think we’ve reached something good in the result. >> >> The selling point for me is this: >> >> // This is allowed since the superclass is `open`. >> class SubclassB : SubclassableParentClass { >> // This is invalid because it overrides a method that is >> // defined outside of the current module but is not `open'. >> override func foo() { } >> >> // This is allowed since the superclass's method is overridable. >> // It does not need to be marked `open` because it is defined on >> // an `internal` class. >> override func bar() { } >> } >> >> This feels super-clean; it gives Library developers `open` for their APIs, >> without confusing app developers, and still requires that sub-classing >> Library developers think about `open`. >> >> Good job, everyone! >> >> Scott >> _______________________________________________ >> swift-evolution mailing list >> [email protected] <mailto:[email protected]> >> https://lists.swift.org/mailman/listinfo/swift-evolution >
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
