Babel: it-N, en-3, fr-1
Yeah no need to rollback, for sure. The fix is also pretty small and self-contained.
Ouch! I'm going to hotfix that - no need to revert that commit in its entirety. I'll also add a selenium test for actions. I think this should be a train blocker given that the form is unusable no matter what.
FTR: CX has its own AF checker, which is probably the "culprit" here.
Examining via the link from the test interface shows that translate_source_text isn't set, while the original abuse log entry does have that variable: https://www.mediawiki.org/wiki/Special:AbuseFilter/examine/log/180272
I've gone crazy with all those globals, back-compat stuff, and reinvented wheels, but tests are working now!
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MediaWikiFarm/+/545263/ seems to mitigate the issue, but, well... The tests are messing up with globals a lot (amongst other things), so something else broke (as expected).
I don't think there's one. Apparently, it was introduced because it was faster than MW's PHPUnit config. But now in 2019 we have MediaWikiUnitTestCase etc., so let's just do what extensions normally do.
Oh, from a quick glance: it doesn't return DUNDEFINED because this way dundefined | true is also true (and we want this). So it should return DUNDEFINED only if either operand is undefined, AND the operator is &. I don't know whether that should be handled by boolOp, or by the parser.
AFPData::boolOp discards the DUNDEFINED value in both operands. I think it should do like other methods, and return DUNDEFINED if either of the operands is undefined, instead. However, I should first understand why I didn't do it at first. We should also add a test like
Mon, Oct 21
I see that the extension has its own phpunit.xml file. I wonder whether we should just get rid of it, given that it's non-standard. Or at least keep it compatible with core's PHPUnit settings, but that sounds challenging...
There's already https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/EventBus/+/544332/, I was waiting for CI to finish before giving a final approve.
Sun, Oct 20
Sat, Oct 19
A reminder for something that puzzled me for a couple of minutes: tests where DatabaseBlock::mReason (note: NOT AbstractBlock::mReason) is accessed directly are likely not failing because a dynamic property is created instead. Nevertheless, this is not a good reason to make the property private, given the breaking change.
Note that I had to temporarily revert the patch above. It made AbstractBlock::mReason private without hard deprecation, and lots of extensions are accessing it directly. I apologize if this could seem an overkill, but better reverted than sorry. It can be re-applied without making the property private if it works for you.
Seems the same as T213982? Long story short, the account creator is an IP address, hence filtering by user (where user = the name of the account being created) won't work. OTOH, the non-existing account has some entries in the AbuseLog because attributing them to the underlying IP would disclose personal info.
Fri, Oct 18
For setExpectedException, the two patches above will remove the last non-SMW usages. For $this->getMock, there's still a bunch of extensions using it. And also, Wikibase is still using the PHPUnit4And6Compat trait because some tests in https://github.com/DataValues/Common use it; I don't know how to tackle those. All of these is what's left before we can drop the trait.
A little update: after the update to phan 2.2.13, taint-check will start taking way more memory. Scanning AbuseFilter with r542757 checked out took 3056MB of memory on my machine. I think after releasing 3.0.0 (or even before), we should start thinking about performance, both time- and memory-wise.
Somewhat related to T177780. I've just found an incomplete comment on that task where I was saying that filtering thank actions in AF would be technically difficult, but didn't say why :(
Thu, Oct 17
Fine by me, thanks!
Wed, Oct 16
Tue, Oct 15
A possible improvement is T235390. While the seccheck job is pretty fast, it consumes roughly 1.5GB of memory for the average extension (way more for core, where it still has to be enabled), and this usage will increase with future phan versions. Merging the two jobs would mean saving that amount of memory and hence freeing up CI resources. I don't know whether that would have a noticeable impact on the runtime, though.
Mon, Oct 14
That's because automatic account creations are not in the recent changes. Hence, the AF cannot pick them. This lack is intentional (they would clog the logs), so I don't see any solution here.
Sun, Oct 13
Uhhhh, I just realized that there's a huge problem in doing this: we'd be requiring two versions of phan at the same time. Right now, it'd be 2.2.13 for mediawiki-phan-config, and 1.3.2 for taint-check.
Moved the 2.1.0 part to T235381 for clarity.