[00:23:55] * AaronSchulz is in the centre of rain system, conveniently localised in CA, harbouring the spectre of drab colours and wetness for days on end. I eagerly await the spendour of more favoured weather... [00:24:00] * AaronSchulz grins back at James_F [00:31:06] * bd808 sends AaronSchulz some spare sunshine from Boise [00:32:17] bd808: most of the north (and upper south) of the states looked frigit last I checked. Is it actually warm too? [00:32:42] its been mid-50's for the last couple of weeks [00:32:58] so not horrible [00:33:24] I think we even had a 70 degree day last week [00:33:42] its spring in the desert though so the temp can swing like crazy [04:27:48] AryehGregor: the deprecated behavior of PHPUnit's code coverage is pretty bad, once we upgrade to PHPUnit 6, I plan on turning that off so we get proper coverage metrics for deprecated things [04:28:31] Also there are some bugs in 5.6+xdebug that I found where code that isn't covered is marked as covered (fixed in 7.0) [04:28:59] https://doc.wikimedia.org/cover/mediawiki-core-php7/ (run with PHP 7) might be more accurate, but it's still using the old PHPUnit [12:11:24] legoktm: What's blocking an upgrade to higher versions of PHPUnit? PHP version requirements? It seems like you could have one set of tests that works with multiple versions of PHPUnit, no? [12:11:57] (Although it could be a bit more burdensome, obviously) [12:12:29] The coverage reporting we have now has silly bugs, like reporting ending braces of functions as not covered in some cases. [12:21:12] We have some back compat issues too that are being worked on [12:22:02] https://gerrit.wikimedia.org/r/#/c/394851/ [12:22:08] https://phabricator.wikimedia.org/T177132 [14:50:03] Why would it be, in a phpunit test, that I call $block-delete() and it is indeed removed from the database, but the user is still blocked vis-a-vis editing other pages? [14:50:33] It seems to work for ApiBlockTest::tearDown. [14:51:28] block stuff cached in the user object? [15:05:21] Okay, but how do I fix it? [15:05:39] (And why doesn't it happen in ApiBlockTest::tearDown?) [15:09:24] presumably invlidating the user cache for that user... and refetching it from the DB [15:09:31] as for apiblocktest [15:09:32] let's have a look [15:11:24] in tearDown it's not doing anything with the user object... Just removing the block to cleanup [15:23:13] Reedy: Okay, but then in subsequent tests, the user isn't actually blocked. But in the test that I'm working on, it remains blocked for later tests in the file. [15:24:09] You're not caching it as a class object are you? [15:27:31] class member [15:28:18] Reedy: https://pastebin.com/wEHP1CCL The block is created in testEditWhileBlocked, the errors occur in testTooBigEdit and testCreateImageRedirectLoggedIn. [15:28:27] Ignore the redeclaration of testTooBigEdit. [15:31:11] Looks to be something to do with $wgUser [15:38:15] $wgUser->invalidateCache() in tearDown? :/ [15:39:42] Reedy: Nope, doesn't help. [15:40:32] clearInstanceCache()? [15:40:38] As that does... $this->mBlockedby = -1; # Unset [15:41:07] Nope. [15:41:12] :/ [15:42:10] Temporary workaround to get my tests to pass for now: make the blocking test the last one in the class. [15:42:58] helpful :/ [15:57:26] Well, the way I look at it, it's the reviewer's problem to decide what to do about it. :) [15:57:55] One of the great advantages of a review-then-commit model, I don't have to actually take responsibility for anything. [16:01:23] It wasn't a dig at you. Just amusement [16:01:38] Someone else will probably have an idea what needs kicking to fix the state issue [16:01:42] Yes, presumably. [16:01:50] One would hope. [16:02:08] At worst, you just Nerd Snipe Tim into dealing with it ;) [16:02:35] Well, anomie is reviewing this, so I'm just putting an XXX comment in the code and will let him look at it. [16:03:25] Hence at worst [16:06:43] AryehGregor: Try self::$users['sysop']->getUser()->clearInstanceCache() after deleting the block? [16:07:26] anomie: Nope. [16:10:27] I'll be back in a bit over an hour. [17:34:32] Hmm, so stylize.php doesn't like this: $this->mergeMwGlobalArrayValue( 'wgHooks', [ 'EditFilterMergedContent' => [ function() { return false; } ] ] ); [17:34:42] Because it says statements and closing braces need to be on their own lines. [17:34:57] Is this really unwanted? It seems a bit silly in this case. [17:35:39] You can ignore the sniff on a line by line basis [17:35:58] It's probably a use case that we've either styled out, or we don't use very often [17:36:44] legoktm: ^ [17:40:45] anomie, cscott https://gerrit.wikimedia.org/r/#/c/420945/ .. followup [18:23:26] Is there a tool that will let me answer a question like "what derived classes of ApiBase do not override getExamplesMessages"? [18:48:32] mmm, do we still need stylize? [18:52:10] the whole repo looks abandonware [18:52:36] I miss styleize.php [19:06:33] I suppose these days we use phpcbf for the things phpcs can fix. [19:07:29] Notice: Cannot access property on non-object in /srv/mediawiki/php-1.31.0-wmf.25/extensions/GlobalBlocking/includes/GlobalBlocking.class.php on line 119 [19:29:24] Notice: Uninitialized string offset: 0 in /srv/mediawiki/php-1.31.0-wmf.26/extensions/CentralAuth/includes/specials/SpecialGlobalGroupMembership.php on line 95 [19:35:18] Hmm. Is it safe to assume the WikimediaEvents extension uses the "release branches" compatibility policy? It doesn't seem to have an owner listed anywhere. [19:47:52] Why do I have to write @covers annotations on all my methods? Why can't the coverage thingie just mark it as covering whatever it covers? [19:49:16] AryehGregor: i think that actually prevents it from marking as covered anything other than the method you name in @covers [19:49:29] so you don't accidentally cover some deep internals [19:50:06] MatmaRex: What's wrong if I do? At the end of the day, that means it's covered, right? [19:50:15] I.e., those code paths are in fact being exercised by the tests. [19:50:35] folks wanted to be stricter, i guess [19:52:41] I think it's so that only code a test case is intentionally testing is marked as covered [19:53:29] imho yeah, if e.g. an API test case performs log in, you aren't really exercising the login functionality with possible scenarios and inputs [19:55:04] anomie: Tbh, I dunno how useful Wikimedia* extensions are having REL1_* branches [19:55:35] no_justification: Aren't the REL branches created automatically now? [19:55:47] No? [19:55:51] I have a script [19:55:54] But it's not automagic [19:56:57] WikimediaEvents has REL branches from 1_22 on. [20:01:09] * MaxSem scratches head [20:11:59] anyone knows python? [20:17:04] MaxSem: Whats up? [20:18:14] eh, in https://github.com/MaxSem/CommonsNotifier/blob/master/commonsbot/state.py#L58 I know the loop is getting executed, however mysql.query() just isn't getting called :O [20:18:58] AryehGregor: https://phabricator.wikimedia.org/T154789 for the closure issue, and you can just put the @covers tag on the whole test class instead of every individual method [20:19:38] i.e. you know a line before the function call is getting executed, but the first line of the function is debug output, and it's silent :headdesk: [20:31:12] no_justification: [20:31:21] https://www.irccloud.com/pastebin/EXVGhYfM/ [20:31:41] why does a function immediately return? [20:32:40] Hm.. should we change our README to use Markdown in mediawiki/core? That would make it work in Gitiles. Right now we link to https://phabricator.wikimedia.org/source/mediawiki/ instead of https://gerrit.wikimedia.org/g/mediawiki/core which makes sense given what it currently looks like [20:32:59] Compared to https://gerrit.wikimedia.org/g/WrappedString though, the Gitiles one seems better integrated and has a working readme [20:50:12] eh, figured out. and why did I use python, anyway? 😥 [21:01:58] because python is awesome [21:02:14] But tbh, haven't done mysql work in it sooo...that's why I had no answer