[swift-evolution] [swift-evolution-announce] [Accepted with Revision] SE-0177: Allow distinguishing between public access and public overridability

[Jordan Rose]

Other than warning on open methods in non-open classes, I can’t think of a good way to avoid this and still preserve the rest of the testing model. After all, @testable allows you to override internal methods too.

(Just to make things clear, @testable only applies to the current file, so it’s possible to have a single test bundle with both black-box and glass-box tests, as long as they’re in different files.)

Jordan


> On Jul 27, 2016, at 17:40, David Owens II via swift-evolution <swift-evolution at swift.org> wrote:
> 
> 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 <rjmccall at apple.com <mailto:rjmccall at apple.com>> wrote:
> 
>>> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> 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 <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> 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
>>>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>>>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>>> 
>>> _______________________________________________
>>> swift-evolution mailing list
>>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>> 
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160727/3fd19a38/attachment.html>