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

[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
comes out of years and years of bad experience with core and extension code
subclassing things in annoying ways that inevitably have to be cleaned up
as a side-task to getting some other feature/refactoring patch to pass CI.
It's a clear way to both document and enforce subclass implementation
points. The only reason not to use it is for tests, and I have removed
"final" before (placed in BagOStuff) when I couldn't come up with another
workaround. Interfaces will not work well for protected methods that need
to be overriden and called by an abstract base class.

If no PHP/PHPUnit fix is coming soon, as a practical matter, I'm sure some
other alternative documentation and naming style pattern could be
standardized so that people actually follow it and don't make annoying and
fragile dependencies.

On Wed, Aug 28, 2019 at 12:30 AM Aryeh Gregor  wrote:

> 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 that facilitates testing.
> In each case we need to make a cost-benefit analysis about what's best
> for the project. The question is whether there's any benefit to using
> final that outweighs the cost to testability.
>
> > And sometimes, methods have to be final.
>
> Why do methods ever "have" to be final? Someone who installs an
> extension accepts that they get whatever behavior changes the
> extension makes. If the extension does something we don't want it to,
> it will either work or not, but that's the extension's problem.
>
> This is exactly the question: why do we ever want methods to be final?
> Is there actually any benefit that outweighs the problems for testing?
>
> > Anyway, some time ago I came across [1], which allows mocking final
> methods
> > and classes. IIRC, it does that by removing the `final` keywords from the
> > tokenized PHP code. I don't know how well it works, nor if it could
> degrade
> > performance, but if it doesn't we could bring it in via composer.
>
> That would be a nice solution if it works well. If someone wants to
> volunteer to try to get it working, then we won't need to have this
> discussion. But until someone does, the question remains.
>
> ___
> Wikitech-l mailing list
> Wikitech-l@lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l



-- 
-Aaron
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 a tendency for a test to needlessly duplicate the
source code by being too strict on expecting exactly which method is called
to the point that it becomes nothing but a more verbose version of the
source code; always requiring a change to both.

Personally, I prefer a style of testing where it providers a simpler view
of the code. More like a manual of how it should be used, and what
observable outcome it should produce.

I think PHPUnit's assertion helpers for things like method()->once() are
quite useful. But, personally, I almost never need them. And in the few
occasions where I did use them, it's never a private, static or final
method (which can't be mocked). In some cases, I accidentally tried to mock
such method and found every time it was either because of pre-existing
technical debt, or because I misunderstood the code, or because I was
testing arbitrary implementation details.

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.

-- Timo
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

[Wikitech-l] [Tech Talks] Sept 4, 2019, 6 PM UTC, Documenting Wikimedia technical projects

[Sarah R]

Hi Everyone,

It's time for Wikimedia Tech Talks 2019 Episode 7! This talk will take
place *Sept 4, 2019 at 6PM UTC*.

*Topic:*  Documenting Wikimedia technical projects

*Speaker:* Sarah R. Rodlund, Technical Writer -- Developer Advocacy

*Summary: *

This talk will discuss what technical writers do and why they are critical
members of the Wikimedia technical community. You will learn more about the
skills needed to be a technical writer and how to build these skills by
participating on Wikimedia and other open source projects.

The talk will also cover some ongoing initiatives to improve technical
documentation for Wikimedia projects.
*YouTube stream for viewers:* https://www.youtube.com/watch?v=GKvyk4VAPb0

During the live talk, you are invited to join the discussion on IRC at
#wikimedia-office

You can watch past Tech Talks here:
https://www.mediawiki.org/wiki/Tech_talks

If you are interested in giving your own tech talk, you can learn more
here:
https://www.mediawiki.org/wiki/Project:Calendar/How_to_schedule_an_event#Tech_talks

Many kindnesses,

Sarah R. Rodlund
Technical Writer, Developer Advocacy

srodl...@wikimedia.org
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 are questionable, then...

>> If somebody is volunteering to do the necessary work in the CI
> infrastructure, fine.
>>
>> To me it just seems like just removing the final modifier is the easier
> and
>> cheaper solution, and doesn't have any big downside.
> 
> It depends on how hard is it to set up the library. Ideally, we only have
> to find a place where to enable it, nothing more. And I also think it won't
> harm existing tests in any way. It doesn't even require a CI change. The
> only possible drawback is performance / opcache integration pointed out
> by @simetrical. Let me try to upload a test change and see how it goes,
> tonight or tomorrow.

Cool, thank you for looking into this!

Sorry if I sounded very negative. I'm trying to be pragmatic and avoid extra
effort where it is not needed. I appreciate your help with evaluating this!

-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
Wikimedia Foundation

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 impossible to write unit tests.
>
> Maybe not impossible with the tool you pointed to. If that thing works, it
> becomes: it requires effort to set up the CI infrastructure to allow this
to
> work, and we don't know who is going to do that, or when.

Ah yes, I'm actually writing with the idea in mind that we'll be able to
set up such a tool. If finals and mocking were mutually exclusive, I'd also
prefer to have better tests.

>> But making methods mockable while also keeping the
>> capability to use finals is even better IMHO.
>
> If that can be made to work, sure. I'm just saying that an inability to
mock far
> outweights the potential benefits of declaring things as final.

Yes, agreed with that.

>> In theory, for sure. But I believe there are lots of occurrences in our
>> code where static methods are not pure functions.
>
> Which indeed is one of the things we are currently trying to fix, because
static
> code can't be mocked.

Indeed, and that's not so bad because it (hopefully) forces people not to
write non-pure static methods.

> Two sides of a coin, I think. Each of them has its benefits and its
> drawbacks, I'd say.
>
> 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.

> If somebody is volunteering to do the necessary work in the CI
infrastructure, fine.
>
> To me it just seems like just removing the final modifier is the easier
and
> cheaper solution, and doesn't have any big downside.

It depends on how hard is it to set up the library. Ideally, we only have
to find a place where to enable it, nothing more. And I also think it won't
harm existing tests in any way. It doesn't even require a CI change. The
only possible drawback is performance / opcache integration pointed out
by @simetrical. Let me try to upload a test change and see how it goes,
tonight or tomorrow.


Il giorno mer 28 ago 2019 alle ore 18:54 Daniel Kinzler <
dkinz...@wikimedia.org> ha scritto:

> 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 analysis.
> >
> > Why not just use final, then?
>
> Because it makes it impossible to write unit tests.
>
> Maybe not impossible with the tool you pointed to. If that thing works, it
> becomes: it requires effort to set up the CI infrastructure to allow this
> to
> work, and we don't know who is going to do that, or when.
>
> >> Method contracts should be enforced by compliance tests. When following
> > these principles, making
> >> methods and classes final has little benefit.
> >
> > Ideally, yes. But I don't think our codebase has enough tests for that.
>
> That's what we are trying to fix, and final stuff is making it hard.
>
> >> Preventing mocking is however a pretty massive cost.
> >
> > Definitely yes. But making methods mockable while also keeping the
> > capability to use finals is even better IMHO.
>
> If that can be made to work, sure. I'm just saying that an inability to
> mock far
> outweights the potential benefits of declaring things as final.
>
> > In theory, for sure. But I believe there are lots of occurrences in our
> > code where static methods are not pure functions.
>
> Which indeed is one of the things we are currently trying to fix, because
> static
> code can't be mocked.
>
> >> with these contracts. Final methods make callers rely on a specific
> >> implementation, which may still end up changing anyway.
> >
> > Two sides of a coin, I think. Each of them has its benefits and its
> > drawbacks, I'd say.
>
> What benefits does it have to bind to a specific implementation that is not
> guaranteed to stay as it is?
>
> >> If I understand correctly, this would break as soon as the mock object
> > hits a
> >> type hint of instanceof check. That won't fly.
> >
> > 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.
>
> If somebody is volunteering to do the necessary work in the CI
> infrastructure, fine.
>
> To me it just seems like just removing the final modifier is the easier and
> cheaper solution, and doesn't have any big downside.
>
> --
> Daniel Kinzler
> Principal Software Engineer, Core Platform
> Wikimedia Foundation
>
> ___
> Wikitech-l mailing list
> 

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

[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 analysis.
> 
> Why not just use final, then?

Because it makes it impossible to write unit tests.

Maybe not impossible with the tool you pointed to. If that thing works, it
becomes: it requires effort to set up the CI infrastructure to allow this to
work, and we don't know who is going to do that, or when.

>> Method contracts should be enforced by compliance tests. When following
> these principles, making
>> methods and classes final has little benefit.
> 
> Ideally, yes. But I don't think our codebase has enough tests for that.

That's what we are trying to fix, and final stuff is making it hard.

>> Preventing mocking is however a pretty massive cost.
> 
> Definitely yes. But making methods mockable while also keeping the
> capability to use finals is even better IMHO.

If that can be made to work, sure. I'm just saying that an inability to mock far
outweights the potential benefits of declaring things as final.

> In theory, for sure. But I believe there are lots of occurrences in our
> code where static methods are not pure functions.

Which indeed is one of the things we are currently trying to fix, because static
code can't be mocked.

>> with these contracts. Final methods make callers rely on a specific
>> implementation, which may still end up changing anyway.
> 
> Two sides of a coin, I think. Each of them has its benefits and its
> drawbacks, I'd say.

What benefits does it have to bind to a specific implementation that is not
guaranteed to stay as it is?

>> If I understand correctly, this would break as soon as the mock object
> hits a
>> type hint of instanceof check. That won't fly.
> 
> 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.

If somebody is volunteering to do the necessary work in the CI infrastructure, 
fine.

To me it just seems like just removing the final modifier is the easier and
cheaper solution, and doesn't have any big downside.

-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
Wikimedia Foundation

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 handler for the file:// protocol, and rely on the fact that
> the PHP engine also uses that handler to read source code files.

I wonder how it interacts with an opcode cache. Is the cache going to
return the cached result based on mtime or whatever, meaning you'll
get a random mix of code with and without final and tests might fail
because they got a cached version of the file that wasn't
de-finalized? Or does it somehow know? (I don't see how it would.)

I filed an issue on this:
https://github.com/dg/bypass-finals/issues/12 Assuming it somehow
works with an opcode cache, it shouldn't have to be a huge performance
impact, because the files shouldn't be parsed often.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 seems to override the
standard PHP handler for the file:// protocol, and rely on the fact that
the PHP engine also uses that handler to read source code files. (Also, I
think it only applies to .php files and not Apache-style .php.en files, but
I guess that’s not a problem for us.)

Am Mi., 28. Aug. 2019 um 16:49 Uhr 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.
>
> > which could even be enforced via static analysis.
>
> Why not just use final, then?
>
> > Method contracts should be enforced by compliance tests. When following
> these principles, making
> > methods and classes final has little benefit.
>
> Ideally, yes. But I don't think our codebase has enough tests for that.
>
> > Preventing mocking is however a pretty massive cost.
>
> Definitely yes. But making methods mockable while also keeping the
> capability to use finals is even better IMHO.
>
> > Static code should only be used for pure functions. That's a very limited
> use case.
>
> In theory, for sure. But I believe there are lots of occurrences in our
> code where static methods are not pure functions.
>
> >> If you want to make sure that any subclass won't ever change the
> >> implementation of a method, and thus all callers know what to expect
> from
> >> calling a final method.
> >> I see finals as a sort of safeguard to help write better code, like e.g.
> >> typehints.
> >
> >This should eb done by documenting contracts, and having tests ensure
> compliance
> >with these contracts. Final methods make callers rely on a specific
> >implementation, which may still end up changing anyway.
>
> Two sides of a coin, I think. Each of them has its benefits and its
> drawbacks, I'd say.
>
> >> IMHO this would be a perfect compromise. I've filed T231419 for that,
> and I
> >> also think that before discussing any further, we should try to see if
> we
> >> can install that tool.
> >
> >If I understand correctly, this would break as soon as the mock object
> hits a
> >type hint of instanceof check. That won't fly.
>
> 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.
>
>
> Il giorno mer 28 ago 2019 alle ore 16:29 Daniel Kinzler <
> dkinz...@wikimedia.org> ha scritto:
>
> > 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 compliance tests. When following these principles,
> > making
> > methods and classes final has little benefit. Preventing mocking is
> > however a
> > pretty massive cost.
> >
> > I'd just remove the final markers. But maybe I'm overlooking
> something?...
> >
> > Am 28.08.19 um 10:38 schrieb 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.
> >
> > Static code should only be used for pure functions. That's a very limited
> > use case.
> >
> > >> Why do methods ever "have" to be final?
> > >
> > > If you want to make sure that any subclass won't ever change the
> > > implementation of a method, and thus all callers know what to expect
> from
> > > calling a final method.
> > > I see finals as a sort of safeguard to help write better code, like
> e.g.
> > > typehints.
> >
> > This should eb done by documenting contracts, and having tests ensure
> > compliance
> > with these contracts. Final methods make callers rely on a specific
> > implementation, which may still end up changing anyway.
> >
> > >
> > >> That would be a nice solution if it works well. If someone wants to
> > >> volunteer to try to get it working, then we won't need to have this
> > >> discussion. But until someone does, the question remains.
> > >
> > > IMHO this would be a perfect compromise. I've filed T231419 for that,
> > and I
> > > also think that before discussing any further, we should try to see if
> we
> > > can install that tool.
> >
> > If I understand correctly, this would break as soon as the mock object
> > hits a
> > type hint of instanceof check. That won't fly.
> >
> >
> > --
> > Daniel Kinzler
> > Principal Software Engineer, Core Platform
> > Wikimedia Foundation
> >
> > 

[Wikitech-l] Upcoming Search Platform Office Hours—September 4th

[Trey Jones]

The Search Platform Team
 usually holds
office hours the first Wednesday of each month. Come talk to us about
anything related to Wikimedia search!


Feel free to add your items to the Etherpad Agenda for the next meeting.


Details for our next meeting:

Date: Wednesday, Sep 4th, 2019

Time: 15:00-16:00 GMT / 08:00-9:00 PDT / 11:00-12:00 EDT / 17:00-18:00 CEST

Etherpad: https://etherpad.wikimedia.org/p/Search_Platform_Office_Hours

Google Meet link: https://meet.google.com/vyc-jvgq-dww


Hope to talk to you in a week!

Trey Jones
Sr. Software Engineer, Search Platform
Wikimedia Foundation
UTC-4 / EDT
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 following
these principles, making
> methods and classes final has little benefit.

Ideally, yes. But I don't think our codebase has enough tests for that.

> Preventing mocking is however a pretty massive cost.

Definitely yes. But making methods mockable while also keeping the
capability to use finals is even better IMHO.

> Static code should only be used for pure functions. That's a very limited
use case.

In theory, for sure. But I believe there are lots of occurrences in our
code where static methods are not pure functions.

>> If you want to make sure that any subclass won't ever change the
>> implementation of a method, and thus all callers know what to expect from
>> calling a final method.
>> I see finals as a sort of safeguard to help write better code, like e.g.
>> typehints.
>
>This should eb done by documenting contracts, and having tests ensure
compliance
>with these contracts. Final methods make callers rely on a specific
>implementation, which may still end up changing anyway.

Two sides of a coin, I think. Each of them has its benefits and its
drawbacks, I'd say.

>> IMHO this would be a perfect compromise. I've filed T231419 for that,
and I
>> also think that before discussing any further, we should try to see if we
>> can install that tool.
>
>If I understand correctly, this would break as soon as the mock object
hits a
>type hint of instanceof check. That won't fly.

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.


Il giorno mer 28 ago 2019 alle ore 16:29 Daniel Kinzler <
dkinz...@wikimedia.org> ha scritto:

> 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 compliance tests. When following these principles,
> making
> methods and classes final has little benefit. Preventing mocking is
> however a
> pretty massive cost.
>
> I'd just remove the final markers. But maybe I'm overlooking something?...
>
> Am 28.08.19 um 10:38 schrieb 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.
>
> Static code should only be used for pure functions. That's a very limited
> use case.
>
> >> Why do methods ever "have" to be final?
> >
> > If you want to make sure that any subclass won't ever change the
> > implementation of a method, and thus all callers know what to expect from
> > calling a final method.
> > I see finals as a sort of safeguard to help write better code, like e.g.
> > typehints.
>
> This should eb done by documenting contracts, and having tests ensure
> compliance
> with these contracts. Final methods make callers rely on a specific
> implementation, which may still end up changing anyway.
>
> >
> >> That would be a nice solution if it works well. If someone wants to
> >> volunteer to try to get it working, then we won't need to have this
> >> discussion. But until someone does, the question remains.
> >
> > IMHO this would be a perfect compromise. I've filed T231419 for that,
> and I
> > also think that before discussing any further, we should try to see if we
> > can install that tool.
>
> If I understand correctly, this would break as soon as the mock object
> hits a
> type hint of instanceof check. That won't fly.
>
>
> --
> Daniel Kinzler
> Principal Software Engineer, Core Platform
> Wikimedia Foundation
>
> ___
> Wikitech-l mailing list
> Wikitech-l@lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l



-- 
https://meta.wikimedia.org/wiki/User:Daimona_Eaytoy
"Daimona" is not my real name -- he/him
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 compliance tests. When following these principles, making
methods and classes final has little benefit. Preventing mocking is however a
pretty massive cost.

I'd just remove the final markers. But maybe I'm overlooking something?...

Am 28.08.19 um 10:38 schrieb 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.

Static code should only be used for pure functions. That's a very limited use 
case.

>> Why do methods ever "have" to be final?
> 
> If you want to make sure that any subclass won't ever change the
> implementation of a method, and thus all callers know what to expect from
> calling a final method.
> I see finals as a sort of safeguard to help write better code, like e.g.
> typehints.

This should eb done by documenting contracts, and having tests ensure compliance
with these contracts. Final methods make callers rely on a specific
implementation, which may still end up changing anyway.

> 
>> That would be a nice solution if it works well. If someone wants to
>> volunteer to try to get it working, then we won't need to have this
>> discussion. But until someone does, the question remains.
> 
> IMHO this would be a perfect compromise. I've filed T231419 for that, and I
> also think that before discussing any further, we should try to see if we
> can install that tool.

If I understand correctly, this would break as soon as the mock object hits a
type hint of instanceof check. That won't fly.


-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
Wikimedia Foundation

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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?

If you want to make sure that any subclass won't ever change the
implementation of a method, and thus all callers know what to expect from
calling a final method.
I see finals as a sort of safeguard to help write better code, like e.g.
typehints.

> That would be a nice solution if it works well. If someone wants to
> volunteer to try to get it working, then we won't need to have this
> discussion. But until someone does, the question remains.

IMHO this would be a perfect compromise. I've filed T231419 for that, and I
also think that before discussing any further, we should try to see if we
can install that tool.

Il giorno mer 28 ago 2019 alle ore 09:30 Aryeh Gregor  ha
scritto:

> 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 that facilitates testing.
> In each case we need to make a cost-benefit analysis about what's best
> for the project. The question is whether there's any benefit to using
> final that outweighs the cost to testability.
>
> > And sometimes, methods have to be final.
>
> Why do methods ever "have" to be final? Someone who installs an
> extension accepts that they get whatever behavior changes the
> extension makes. If the extension does something we don't want it to,
> it will either work or not, but that's the extension's problem.
>
> This is exactly the question: why do we ever want methods to be final?
> Is there actually any benefit that outweighs the problems for testing?
>
> > Anyway, some time ago I came across [1], which allows mocking final
> methods
> > and classes. IIRC, it does that by removing the `final` keywords from the
> > tokenized PHP code. I don't know how well it works, nor if it could
> degrade
> > performance, but if it doesn't we could bring it in via composer.
>
> That would be a nice solution if it works well. If someone wants to
> volunteer to try to get it working, then we won't need to have this
> discussion. But until someone does, the question remains.
>
> ___
> Wikitech-l mailing list
> Wikitech-l@lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l



-- 
https://meta.wikimedia.org/wiki/User:Daimona_Eaytoy
"Daimona" is not my real name -- he/him
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

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

[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 that facilitates testing.
In each case we need to make a cost-benefit analysis about what's best
for the project. The question is whether there's any benefit to using
final that outweighs the cost to testability.

> And sometimes, methods have to be final.

Why do methods ever "have" to be final? Someone who installs an
extension accepts that they get whatever behavior changes the
extension makes. If the extension does something we don't want it to,
it will either work or not, but that's the extension's problem.

This is exactly the question: why do we ever want methods to be final?
Is there actually any benefit that outweighs the problems for testing?

> Anyway, some time ago I came across [1], which allows mocking final methods
> and classes. IIRC, it does that by removing the `final` keywords from the
> tokenized PHP code. I don't know how well it works, nor if it could degrade
> performance, but if it doesn't we could bring it in via composer.

That would be a nice solution if it works well. If someone wants to
volunteer to try to get it working, then we won't need to have this
discussion. But until someone does, the question remains.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l