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