While arguably true, that's a philosophical debate. There are plenty of reasons to use @testable, and if that's what people are using, then I think there is a valid concern here.
Sent from my iPhone > On Jul 27, 2016, at 5:22 PM, John McCall <[email protected]> wrote: > >> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution >> <[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? > > A "black box" unit test emulating consumer behavior has no business using a > @testable import. It should just use the external API of the library. > > John. > >> >> 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]> 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] >>> https://lists.swift.org/mailman/listinfo/swift-evolution >> >> _______________________________________________ >> swift-evolution mailing list >> [email protected] >> https://lists.swift.org/mailman/listinfo/swift-evolution >
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
