[Wikitech-l] New `composer phpunit:entrypoint` command (phpunit.php deprecated)
TL;DR: The tests/phpunit/phpunit.php script is deprecated. Use `composer phpunit:entrypoint` instead. Thus far there have been three ways to run PHPUnit tests in MediaWiki: 1. Via simple composer command: `composer phpunit` / `vendor/bin/phpunit` (only for non-DB tests) 2. Via composer entrypoint: `composer phpunit:entrypoint` 3. Via custom PHP script: `php tests/phpunit/phpunit.php` The third method is now deprecated [1]. The `composer phpunit:entrypoint` command, previously pointing to the phpunit.php script, now uses `vendor/bin/phpunit`, specifying a config file that can be used with integration and database tests. Where you would previously use phpunit.php, you should now use `composer phpunit:entrypoint`. The rationale behind this change is to make it easier for developers to run PHPUnit tests, without having to determine what entry point they should use for a given test. The previous setup also made it more difficult to configure IDEs, because you had to switch between the available entry points to make sure that tests were run with the expected config. After this change, the only difference between `composer phpunit` and `composer phpunit:entrypoint` is the config file they use (phpunit.xml.dist and tests/phpunit/suite.xml, respectively). Note that we are also planning to phase out the latter; you can follow the conversation in [2]. When that happens, we will truly have a single entry point for all PHPUnit tests! The switch from phpunit.php to composer comes with a small but potentially significant difference: LocalSettings.php is no longer loaded in the global scope. This should work transparently to you. If one of your settings appear to have no effect, please report this on Phabricator [1]. The main scenario where these issues may arise is if your configuration variable is read early during MediaWiki setup, for instance in an extension function [3]. You can temporarily workaround this by making the config variable explicitly global in the place where it's being set, with `global $wgMyGlobal;`. Huge thanks to Timo, James F., Kosta, Antoine, Umherirrender, and everyone who helped with this long-overdue change. [1] - https://phabricator.wikimedia.org/T90875 [2] - https://phabricator.wikimedia.org/T227900 [3] - https://www.mediawiki.org/wiki/Manual:$wgExtensionFunctions -- 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 To unsubscribe send an email to wikitech-l-le...@lists.wikimedia.org https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/
[Wikitech-l] Standardizing exception handling in MediaWiki
Hi everyone, Today I created a task proposing that exception handling and annotations be standardized in MediaWiki: https://phabricator.wikimedia.org/T321683. This would affect all MediaWiki code written in PHP (core, extensions, and skins), and the standardization itself will require a lot of effort. Therefore, I invite you to share any questions, thoughts, or concerns that you may have on the task. Thank you, -- 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 To unsubscribe send an email to wikitech-l-le...@lists.wikimedia.org https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/
[Wikitech-l] Removing methods in the Article class without deprecation
Hello all, Next week we will remove the following methods without deprecation: - Article::delete() - Article::confirmDelete() - ImagePage::delete() This is part of an ongoing refactoring of deletion-related code. The methods above are currently bloated: beyond deleting an article, they're responsible for building the UI, checking permissions, handling errors etc. The relevant UI-related code is being moved to DeleteAction, so use that if you need to do something with the UI. Maintaining BC in the old methods is challenging, due to how many things this code does, and how much code is being moved around. According to codesearch, these methods are only used by a test in WikibaseLexeme (which has been temporarily disabled). Thus, we decided to remove them without deprecation after this week's train. Please reply to this email if you're affected by this and don't know how to fix. Best regards, -- 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 To unsubscribe send an email to wikitech-l-le...@lists.wikimedia.org https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/
[Wikitech-l] Removal of custom CLI options for the PHPUnit runner
Hi all, (If you don't locally run PHPUnit tests on MediaWiki, you can skip this notice.) In order to simplify our PHPUnit configuration,[0] the following CLI options for tests/phpunit/phpunit.php were replaced by environment variables in a patch that just landed in the HEAD branch: * --wiki=foo => PHPUNIT_WIKI=foo * --use-normal-tables => PHPUNIT_USE_NORMAL_TABLES=1 * --reuse-db => PHPUNIT_REUSE_DB=1 * --use-filebackend=foo => PHPUNIT_USE_FILEBACKEND=foo * --use-bagostuff=foo => PHPUNIT_USE_BAGOSTUFF=foo * --use-jobqueue=foo => PHPUNIT_USE_JOBQUEUE=foo Furthermore, some of these options are being considered for removal, so if you regularly use them, please raise your voice on the task. Thank you, and thanks to James F, Kosta and Timo for your help with moving this forward. [0] - See https://phabricator.wikimedia.org/T90875 for details -- 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 To unsubscribe send an email to wikitech-l-le...@lists.wikimedia.org https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/
Re: [Wikitech-l] Problems with the CI
Indeed. I should also note that the issue reported at https://phabricator.wikimedia.org/T280064 is about xdebug not being found. I don't have an explanation for this; perhaps something was missed while switching the job to PHP 7.3. OTOH, my proposed solution will make it possible to use pcov instead of xdebug, thus basically ignoring the current issue with xdebug. The rationale being, I don't think it's necessary to unbreak xdebug support if we're migrating away from it anyway. Il giorno mer 14 apr 2021 alle ore 07:05 Kunal Mehta ha scritto: > Hi, > > On 4/13/21 7:47 PM, Zoran Dori wrote: > > > > I would like to know why is this happening. > > See https://phabricator.wikimedia.org/T280064 and > https://phabricator.wikimedia.org/T234020#6989290. Daimona has proposed > https://gerrit.wikimedia.org/r/c/integration/config/+/678383/ to > hopefully fix it. > > -- Legoktm > > ___ > 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
[Wikitech-l] Grant request for an AbuseFilter overhaul
Hello everyone! I have submitted a grant proposal on meta for overhauling and modernizing the AbuseFilter extension [1]. The goal of this proposal is to make the extension easier to maintain, with the side effects of simplifying future maintenance and easing feature additions. The grant will also cover a bugfix part and the addition of a community-requested feature. You can find the request here: https://meta.wikimedia.org/wiki/Grants:Project/Daimona_Eaytoy/AbuseFilter_overhaul If you're interested in this proposal, I invite you to endorse it on meta at the page linked above. If you have any questions, you can use the talk page on meta. Thank you for your time! [1] - https://www.mediawiki.org/wiki/Extension:AbuseFilter -- 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] Dropping PHPUnit 4 support
Thanks, Amir! That's a great idea. My regexps were a bit naive and there's still some space for improvement, so I'll comment about that on the gist. As for removing the PHPUnit4And6Compat trait: I think we should do that regardless of this migration. In fact, I'd like to draw some attention to T234721. Basically, any test using the trait directly does so because it extends PHPUnit\Framework\TestCase. And in turn, this happens because MediaWikiUnitTestCase validates the directory structure, but some repos do not comply. Hence, we should implement a way to use MediaWikiUnitTestCase even with non-standard directories (3 proposals on phab). Then we could migrate everything with another script. This will also make it easier to transition to PHPUnit 8, if we want to add another backward/forward-compat layer. Il giorno sab 5 ott 2019 alle ore 23:58 Amir Sarabadani ha scritto: > Thanks Daimona! > In the spirit of automating everything including ourselves, I wrote a quick > python script using Daimon's regexes to run on codebases and it tries to > clean as much as possible for you. > https://gist.github.com/Ladsgroup/bfb4f4b66384efb8b8d3efea338b6e6b > (After running this, you should run composer fix to fix phpcs issues) > > Feel free to change the gist and put it in comment, so I fix it. > > On Fri, Oct 4, 2019 at 1:58 PM Daimona wrote: > > > Hello all, > > Following our migration away from HHVM, we're now also requiring PHPUnit > 6+ > > [0]. Previously, we were allowing PHPUnit 4 as an option, but that > version > > was EOLed in February 2017 [1]. > > > > MediaWiki core provides a backward/forward-compatible layer [2] to allow > > working with both versions. However, that trait will be emptied and > removed > > soon, partly because we want to move to PHPUnit 8. > > > > Hence, if you have a local install of PHPUnit 4, you won't be able to use > > it anymore. I also invite you to check whether any of your repos has an > > explicit PHPUnit dependency in composer.json [3]. If it does, you can now > > remove PHPUnit 4 from it. > > > > Thank you for your collaboration! > > > > [0] - https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/426058/ > > [1] - https://phpunit.de/supported-versions.html > > [2] - > > > > > https://gerrit.wikimedia.org/g/mediawiki/core/+/e26299b1d14feb341ff5752696f8a24045df9838/tests/phpunit/PHPUnit4And6Compat.php > > [3] - List of repos that do have it: > > > > > https://codesearch.wmflabs.org/search/?q=%22phpunit%5C%2Fphpunit%22%3A=nope=%5Ecomposer%5C.json%24= > > > > -- > > 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 > > > > -- > Amir (he/him) > ___ > 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
[Wikitech-l] Dropping PHPUnit 4 support
Hello all, Following our migration away from HHVM, we're now also requiring PHPUnit 6+ [0]. Previously, we were allowing PHPUnit 4 as an option, but that version was EOLed in February 2017 [1]. MediaWiki core provides a backward/forward-compatible layer [2] to allow working with both versions. However, that trait will be emptied and removed soon, partly because we want to move to PHPUnit 8. Hence, if you have a local install of PHPUnit 4, you won't be able to use it anymore. I also invite you to check whether any of your repos has an explicit PHPUnit dependency in composer.json [3]. If it does, you can now remove PHPUnit 4 from it. Thank you for your collaboration! [0] - https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/426058/ [1] - https://phpunit.de/supported-versions.html [2] - https://gerrit.wikimedia.org/g/mediawiki/core/+/e26299b1d14feb341ff5752696f8a24045df9838/tests/phpunit/PHPUnit4And6Compat.php [3] - List of repos that do have it: https://codesearch.wmflabs.org/search/?q=%22phpunit%5C%2Fphpunit%22%3A=nope=%5Ecomposer%5C.json%24= -- 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
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 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 > about other classes' behavior. > > This is not possible at all if any method is declared final. As soon > as the class under test calls a final method, you cannot mock the > object. This is without any use of expects() or with() -- even just > method()->willReturn(). > > > 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. > > The idea of good unit tests is to allow refactoring without having to > worry too much that you're accidentally changing observable behavior > in an undesired way. Ideally, then, any observable behavior should be > tested. Changes in source code that don't affect observable behavior > will never necessitate a change to a test, as long as the test doesn't > cheat with TestingAccessWrapper or such. > > This includes tests for corner cases where the original behavior was > never considered or intended. This is obviously less important to test > than basic functionality, but in practice, callers often accidentally > depend on all sorts of incidental implementation details. Thus > ideally, they should be tested. If the test needs to be updated, that > means that some caller somewhere might break, and that should be taken > into consideration. > > On Thu, Aug 29, 2019 at 1:12 AM Aaron Schulz > wrote: > > Interfaces will not work well for protected methods that need > > to be overriden and called by an abstract base class. > > If you have an interface that the class implements, then it's possible > to mock the interface instead of the class, and the final method > problem goes away. Of course, your "final" is then not very useful if > someone implements the interface instead of extending the class, but > that shouldn't happen if your base class has a lot of code that > subclasses need. > > On Thu, Aug 29, 2019 at 10:37 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 > worry about > > WANObjectCache or its final methods at all. > > Any interface would solve the problem, even if it was just a copy of > all the public methods of WANObjectCache. That would be inelegant, but > another possible solution if we want to keep using final. > > ___ > 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
>> 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 implementa
Re: [Wikitech-l] Declaring methods final in classes
>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
> 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
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 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. [1] - https://github.com/dg/bypass-finals Il giorno martedì 27 agosto 2019, Aryeh Gregor ha scritto: > 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 we really want these methods to be marked final for some reason, the > workaround for PHP is to make an interface that has all the desired > methods, have the class implement the interface, and make type hints all > refer to the interface instead of the class. But if there's no good reason > to declare the methods final to begin with, it's simplest to just drop it. > ___ > 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] Abusefilter and Content Translator
For what concerns the AbuseFilter part, it sounds like some flaky on-wiki filter. However, I need links to check that: for instance, a few links to the users who triggered the filter (and the wiki where the filter was triggered). Il giorno dom 28 apr 2019 alle ore 11:01 ha scritto: > Hi all, > > We are doing an edit-a-thon and some participants are using the content > translator with some complicated issues. > > > > The main one is that the content translator is now integrating Google > translator but when a participant publishes the translation, the > AbuseFIlter > says that it is an automatic translation from Google even if the > participant > has modified the text. > > > > This happens specifically in the Chinese Wikipedia. Is that normal? > > > > We have seen that modifying the wiki links and the external references it > solves the problem, but in other linguistic versions the Content Translator > does it automatically. > > > > In addition it doesn’t work for tables and for infoboxes. > > > > May someone confirm that these issues are connected with some bugs and that > we are not doing some mistakes? > > > > Kind regards > > > > --- > > Ilario Valdelli > > Education Program manager and community liaison > > Wikimedia CH > Verein zur Förderung Freien Wissens > Association pour l’avancement des connaissances libre > Associazione per il sostegno alla conoscenza libera > Switzerland - 8008 Zürich > > Wikipedia: <https://meta.wikimedia.org/wiki/User:Ilario> Ilario > <http://www.wikimedia.ch/> http://www.wikimedia.ch > > > > ___ > 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] Thank you Tuesday
Thanks for the thought! I'm delighted to be part of this big family, where I've got to know so many amazing people. I'd like to seize the opportunity to thank some of you, but I really can't pick. Y'all are awesome! Il giorno lun 15 apr 2019 alle ore 02:19 Krinkle ha scritto: > (A few days late, or a day early.) > > Thank to Ebe123 and Daimona Eaytoy for their work on the Score and > AbuseFilter extensions. For years they've been working on these code bases > and consistently very responsive to bug reports, maintenance inquiries, and > offering help to other interested contributors. > > Thanks for being awesome! > > -- Timo > ___ > 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] Code sniff for verbose conditionals
Thanks for the change! I just realized that my patch has another problem: when checking assignments, it should also check that the receiver is the same in both branches, and this would avoid the case in LoadBalancer. I also find excessive the case in GlobalFunctions, and that would be solved by implementing a check on the complexity of the 'if' condition, as proposed in gerrit comments. Ideas on how to perform such check are welcome. Il giorno lun 11 feb 2019 alle ore 19:42 Bartosz Dziewoński < matma@gmail.com> ha scritto: > On 2019-02-11 18:42, Daimona wrote: > > Hi, > > All patches in the codesniffer repo have a sample run against mwcore set > up > > in CI. As can be seen in [0], the current version is triggered 13 times > by > > MW core. No idea about extensions, though. > > Daimona > > > > [0]: > > > https://integration.wikimedia.org/ci/job/mw-tools-codesniffer-mwcore-testrun/966/console > > Thanks for that link. I looked at them and submitted a change (please do > not merge it) to demonstrate what changes this would require: > https://gerrit.wikimedia.org/r/c/mediawiki/core/+/489759 > > In my opinion most of these changes are clear improvement or harmless, > except for the pattern in LBFactorySimple.php/LoadBalancer.php, which is > a little tricky and probably clearer in the original version. > > -- > Bartosz Dziewoński > > ___ > 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] Code sniff for verbose conditionals
Hi, All patches in the codesniffer repo have a sample run against mwcore set up in CI. As can be seen in [0], the current version is triggered 13 times by MW core. No idea about extensions, though. Daimona [0]: https://integration.wikimedia.org/ci/job/mw-tools-codesniffer-mwcore-testrun/966/console Il giorno lun 11 feb 2019 alle ore 17:49 Kosta Harlan ha scritto: > Hi Daimona, > > Thanks for working on this. Have you run this against sniff against mw > core or extensions? If so it would be useful to look at examples of what > it’s catching in existing code. > > Kosta > > > On Feb 9, 2019, at 9:26 PM, Stas Malyshev > wrote: > > > > Hi! > > > >> I was saying, you can find several examples of wrong and correct code at > >> [1]. > >> Thiemo pointed out that this patch could encourage to write shorter and > >> less readable code (you can find his rationale in Gerrit comments), and > I > >> partly agree with him. My proposal is to only trigger the sniff if the > "if" > >> conditions are "short" enough. Which could mean either a single > condition, > >> shorter than xxx characters etc. > >> We're looking for some feedback to know whether you would deem this > feature > >> useful, and how to tweak it in order to avoid encouraging bad code. > >> Thanks to you all, and again - sorry for the half message. > > > > This looks useful. I think PHPStorm has this check built in, but having > > it in sniffs too wouldn't be a bad thing. I've seen such things happen > > when refactoring code. > > > > -- > > Stas Malyshev > > smalys...@wikimedia.org > > > > ___ > > Wikitech-l mailing list > > Wikitech-l@lists.wikimedia.org > > https://lists.wikimedia.org/mailman/listinfo/wikitech-l > > > ___ > 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 (he/him) ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Code sniff for verbose conditionals
(Pardon, I hit "send" while typing :/) I was saying, you can find several examples of wrong and correct code at [1]. Thiemo pointed out that this patch could encourage to write shorter and less readable code (you can find his rationale in Gerrit comments), and I partly agree with him. My proposal is to only trigger the sniff if the "if" conditions are "short" enough. Which could mean either a single condition, shorter than xxx characters etc. We're looking for some feedback to know whether you would deem this feature useful, and how to tweak it in order to avoid encouraging bad code. Thanks to you all, and again - sorry for the half message. Daimona [0] - https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/codesniffer/+/486813/ [1] - https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/codesniffer/+/486813/16/MediaWiki/Tests/files/ControlStructures/pointless-conditional.php Il giorno sab 9 feb 2019 alle ore 12:30 Daimona ha scritto: > Currently, there's a patch under review [0] to enable a PHP sniff which > would detect so called "Pointless conditionals". These are conditionals > (if...else or ternary) where both branches only assign or return a boolean > value, and which could potentially one-lined using the if condition. T > ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
[Wikitech-l] Code sniff for verbose conditionals
Currently, there's a patch under review [0] to enable a PHP sniff which would detect so called "Pointless conditionals". These are conditionals (if...else or ternary) where both branches only assign or return a boolean value, and which could potentially one-lined using the if condition. T ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] What would you like to see in gerrit?
+1 to Lucas' proposal, it should be possible to blame Jenkins within recheck comments. There's also another minor bug I encountered several times, and I'm unsure if it's already been fixed in 2.16. Have 2 different changes, A and B. If I write A's change ID in the commit message of B, then the UI will show that A is "needed-by" B, and this could lead to confusion especially if the dependency is the other way around, i.e. A depends-on B. And, BTW, thanks for your work on gerrit! Daimona Example A: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/481549/ Example B: https://gerrit.wikimedia.org/r/#/c/integration/config/+/481570/ Il giorno mar 29 gen 2019 alle ore 11:21 Lucas Werkmeister < lucas.werkmeis...@wikimedia.de> ha scritto: > I wonder if it would be possible to allow comments with “recheck” commands? > Often I want to do something like “recheck because CI failed due to T”, > but if I send it as one review then it will not be recognized as a recheck. > Triggering a recheck any time the word “recheck” appears anywhere in a > review would probably cause too many false positives, but I think accepting > any review beginning with that word could be a good middle ground. (I’m not > sure if this would also be useful with other commands like “check > experimental”.) > > Am Di., 29. Jan. 2019 um 01:26 Uhr schrieb Paladox via Wikitech-l < > wikitech-l@lists.wikimedia.org>: > > > Hi, what would you like to see in gerrit or improved? I've been working > > been working on developing a plugin that pull's in zuul's status into > > PolyGerrit. See the running demo at https://imgur.com/a/uBk2oxQ . Im > also > > planning on adding "recheck" and "check experimental" as buttons to > > PolyGerrit's ui to improve CI. This will help new users who can recheck > > (and existing users that either forgot they can type this or haven't > > learned yet). > > Note that i cannot promise that anything suggested in this thread will be > > worked on, but i can try my best. > > See tasks https://phabricator.wikimedia.org/T214068 and > > https://phabricator.wikimedia.org/T214631 . > > > > > > > > > > > > > > > > ___ > > Wikitech-l mailing list > > Wikitech-l@lists.wikimedia.org > > https://lists.wikimedia.org/mailman/listinfo/wikitech-l > > > > -- > Lucas Werkmeister > Full Stack Developer > > Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin > Phone: +49 (0)30 219 158 26-0 > https://wikimedia.de > > Imagine a world in which every single human being can freely share in the > sum of all knowledge. Help us to achieve our vision! > https://spenden.wikimedia.de > > Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V. > Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter > der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für > Körperschaften I Berlin, Steuernummer 27/029/42207. > ___ > Wikitech-l mailing list > Wikitech-l@lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Builds failing on wmf-quibble-core-vendor-mysql-hhvm-docker job
Already filed https://phabricator.wikimedia.org/T214804, this hurts a lot. Daimona Il giorno dom 27 gen 2019 alle ore 20:48 Derick Alangi < alangider...@gmail.com> ha scritto: > Hi everyone, > > Recently, builds are failing on wmf-quibble-core-vendor-mysql-hhvm-docker > job. Seems this is affecting all patches and started hours ago. For > example; > > - > > https://integration.wikimedia.org/ci/job/wmf-quibble-core-vendor-mysql-hhvm-docker/8202/console > > Per this: > > https://integration.wikimedia.org/ci/job/wmf-quibble-core-vendor-mysql-hhvm-docker/ > , > last successful build was 22hrs ago (as at now) meaning this as been > happening for a while, any help please? > > Thanks! > *--* > *Derick* > ___ > Wikitech-l mailing list > Wikitech-l@lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
