Re: [Wikitech-l] Declaring methods final in classes

2019-09-01 Thread Krinkle
On Sun, Sep 1, 2019 at 12:40 PM Aryeh Gregor wrote: > On Fri, Aug 30, 2019 at 10:09 PM Krinkle wrote: > > For anything else, it doesn't really work in my experience because > PHPUnit > > won't actually provide a valid implementation of the interface. It > returns > > null for anything, which is

Re: [Wikitech-l] Declaring methods final in classes

2019-09-01 Thread Aryeh Gregor
On Fri, Aug 30, 2019 at 10:09 PM Krinkle wrote: > For anything else, it doesn't really work in my experience because PHPUnit > won't actually provide a valid implementation of the interface. It returns > null for anything, which is usually not a valid implementation of the > contract the class

Re: [Wikitech-l] Declaring methods final in classes

2019-08-30 Thread Krinkle
On Thu, Aug 29, 2019 at 1:46 PM Aryeh Gregor wrote: > On Thu, Aug 29, 2019 at 1:02 AM Krinkle wrote: > > What did you want to assert in this test? > > In a proper unit test, I want to completely replace all non-value > classes with mocks, so that they don't call the actual class' code. > This

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread Aryeh Gregor
On Thu, Aug 29, 2019 at 5:30 PM Daniel Kinzler wrote: > But subclassing across module boundaries should be restricted to classes > explicitly documented to act as extension points. If we could enforce this > automatically, that would be excellent. Well, for classes that aren't supposed to be

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread Daniel Kinzler
Am 29.08.19 um 15:31 schrieb Adam Wight: > This is a solution I'd love to see explored. Subclassing is now considered > harmful, and it would be possible to refactor existing cases to use > interfaces, traits, composition, etc. I wouldn't say subclassing is considered harmful in all cases. In

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread Adam Wight
On Wed, Aug 28, 2019 at 4:29 PM Daniel Kinzler wrote: > Subclassing should be very limited anyway, and even more limited across > module > boundaries [...] This is a solution I'd love to see explored. Subclassing is now considered harmful, and it would be possible to refactor existing cases

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread Daimona
Note that https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/533067/ is now ready for review, and works as expected. That could make this discussion unnecessary. Il giorno gio 29 ago 2019 alle ore 14:46 Aryeh Gregor ha scritto: > On Thu, Aug 29, 2019 at 1:02 AM Krinkle wrote: > > What did

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread Aryeh Gregor
On Thu, Aug 29, 2019 at 1:02 AM Krinkle wrote: > What did you want to assert in this test? In a proper unit test, I want to completely replace all non-value classes with mocks, so that they don't call the actual class' code. This way I can test the class under test without making assumptions

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread David Causse
On Thu, Aug 29, 2019 at 9:36 AM Daniel Kinzler wrote: > > Narrow interfaces help with that. If we had for instance a cache interface > that > defined just the get() and set() methods, and that's all the code needs, > then we > can just provide a mock for that interface, and we wouldn't have to

Re: [Wikitech-l] Declaring methods final in classes

2019-08-29 Thread Daniel Kinzler
> As such, to perhaps help with the conversation, I'd like to have a > practical example we can look at and compare potential solutions. Perhaps > from WANObjectCache, or perhaps with something else. Simetrical can speak on the concrete use case, but I'd like to give my thoughts on mocking

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Aaron Schulz
Well, changing something in core and breaking a production extenison doing something silly can't be waived away with "it's the extension's problem" ;) I mostly use "final" to enforce a delegation pattern, where only certain key bits of functionality should be filled in by subclasses. It mostly

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Krinkle
On Tue, Aug 27, 2019 at 6:55 PM Aryeh Gregor wrote: > I see that in some classes, like WANObjectCache, most methods are declared > final. Why is this? [..] > > The problem is that PHPUnit mocks can't touch final methods. [..] What did you want to assert in this test? I find there is sometimes

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Daniel Kinzler
>> What benefits does it have to bind to a specific implementation that is > not >> guaranteed to stay as it is? > > If used properly, the final keyword should be for immutable > implementations. Otherwise, it could be a questionable use case. I think all the current uses of "final" in MW core

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Daimona
>> I agree, but it doesn't offer a strict guarantee. > > Do we need a strict guarantee more than we need unit tests? I think we need both. Or rather, we need unit tests more, but if that doesn't exclude using finals, we can have both. >> Why not just use final, then? > > Because it makes it

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Daniel Kinzler
Am 28.08.19 um 16:48 schrieb Daimona: >> Subclassing should be very limited anyway, and even more limited across > module >> boundaries > > I agree, but it doesn't offer a strict guarantee. Do we need a strict guarantee more than we need unit tests? >> which could even be enforced via static

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Aryeh Gregor
On Wed, Aug 28, 2019 at 7:24 PM Lucas Werkmeister wrote: > As far as I can tell, it actually strips final tokens from *any* PHP file > that’s read, including by application code. Yes, but only if you turn it on, and we'd only turn it on for tests. > It seems to override the > standard PHP

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Lucas Werkmeister
> > No, that's only what happens with mockery. The tool I found just strips > 'final' keywords from the PHP code - I believe, I still haven't looked at > the implementation. As far as I can tell, it actually strips final tokens from *any* PHP file that’s read, including by application code. It

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Daimona
>Subclassing should be very limited anyway, and even more limited across module >boundaries I agree, but it doesn't offer a strict guarantee. > which could even be enforced via static analysis. Why not just use final, then? > Method contracts should be enforced by compliance tests. When

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Daniel Kinzler
I see no good use for final methods or classes. Or rather: I see a very limited benefit and a pretty massive cost. Subclassing should be very limited anyway, and even more limited across module boundaries, which could even be enforced via static analysis. Method contracts should be enforced by

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Daimona
> I don't like these limitations either, but testing is an integral part > of development, and we need to code in a way that facilitates testing. This is especially true for e.g. static methods, but here we'd be renouncing to a possibly useful feature. > Why do methods ever "have" to be final?

Re: [Wikitech-l] Declaring methods final in classes

2019-08-28 Thread Aryeh Gregor
On Tue, Aug 27, 2019 at 11:53 PM Daimona wrote: > Personally, I don't like these limitations in PHPUnit and the like. IMHO, > they should never be a reason for changing good code. I don't like these limitations either, but testing is an integral part of development, and we need to code in a way

Re: [Wikitech-l] Declaring methods final in classes

2019-08-27 Thread Máté Szabó
Hey, My understanding is that mocking final methods and types is a limitation that is not specific to just PHPUnit, or indeed to PHP itself. Mockery, another established PHP mock object framework, relies on a workaround for mocking final methods and types that prevents testing code with type

Re: [Wikitech-l] Declaring methods final in classes

2019-08-27 Thread Daimona
Personally, I don't like these limitations in PHPUnit and the like. IMHO, they should never be a reason for changing good code. And sometimes, methods have to be final. I don't know about the specific case, though. Anyway, some time ago I came across [1], which allows mocking final methods and

[Wikitech-l] Declaring methods final in classes

2019-08-27 Thread Aryeh Gregor
I see that in some classes, like WANObjectCache, most methods are declared final. Why is this? Is it an attempt to optimize? The problem is that PHPUnit mocks can't touch final methods. Any ->method() calls that try to do anything to them silently do nothing. This makes writing tests harder. If