[00:07:43] James_F: ok, the proposed patch on github appears to work, so we just need hashar to tell us how to deploy it :) [00:08:46] legoktm: "Just" monkey-patch jenkins? Ha. (I guess we'll need to build a custom .deb or whatever?) [00:09:06] I'm not sure how plugins are handled [00:09:47] I looked through the settings and didn't see any "disable validation" option or anything [00:24:05] you build the plugin then hashar could deploy it [00:24:44] so it's a hdi file i think or hdp [01:42:26] so Wikibase is using Hamcrest asserts, which don't count towards PHPUnit asserts, so it thinks its risky [01:43:36] Hamcrest sounds like rodent heraldry. https://i.imgur.com/6wSFhhV.png [01:43:54] LOL [01:44:04] I think we need something like https://gist.github.com/dharkness/3965902 [02:06:01] Krinkle, James_F: so https://gerrit.wikimedia.org/r/#/c/425942/ appears to work, not sure where it should go though...MediaWikiTestCase? another trait? [02:06:43] legoktm: Hm.. not sure I understand. [02:06:50] legoktm: MatcherAssert::resetCount(); in a trait? [02:07:06] the setUp()/tearDown() parts [02:07:19] why does it fail? [02:07:24] like, what changed? [02:08:04] PHPUnit 6 marks tests as risky/failed if they don't have any assertions [02:08:18] these tests are using Hamcrest assertions (some other library) instead of PHPUnit ones [02:12:08] Hm.. [02:13:56] legoktm: How would the trait work [02:14:17] I mean, you'd have to replace the 1 line with a line that calls it explicitly right? Assuming traits don't sit between parent:: automaticaly [02:14:45] hamcrest is specified in core? [02:14:46] Ugh. [02:15:20] Krinkle: https://paste.fedoraproject.org/paste/RFZ5zncdlL2rKqobaVMmHQ/raw [02:15:48] Right, but if you use it and you have your own setup, only one can win, right? [02:16:04] I don't think it merges into parent:: or something [02:16:31] I think it would do local setUp() -> trait setUp() -> inherited setUp() ? [02:16:34] I'm not sure, let me test [02:16:41] I don't think trait(s) become a parent [02:17:02] They mixin methods, that can be called. [02:17:17] So it'd be `use xTrait` and then adding $this->xSetup(); from setup [02:18:03] So the only problem is when a test uses hamcrest, and only hamcrest, given it'd produce 0 assertions. [02:18:57] https://3v4l.org/dHtl3 looks like the trait is being skipped [02:19:20] Yeah [02:19:50] I'd expect Hamcrest to provide a trait that would be used in classes that use it [02:20:09] and it'd provide a $this->assertHamcrestThat [02:20:12] or some such. [02:21:03] Which upon call, increments phpunit's with +1. [02:21:04] hmm, it would be pretty doable to write that... [02:21:14] That woudl require updating all the assert calls though [02:21:27] I think it's just Wikibase related extensions [02:21:55] https://codesearch.wmflabs.org/search/?q=%5CtassertThat&i=nope&files=&repos= [02:22:00] YEah [02:22:05] And their 3 billion test methods. [02:22:22] So, still a lot. [02:22:41] But it seems Wikibase doesn't use Hamcrest that much [02:22:42] coo [02:22:43] l [02:56:45] Krinkle: https://gerrit.wikimedia.org/r/#/c/425947/ [02:56:53] waiting for tests to run and say it works [02:58:09] legoktm[SJS]: Will still need a Jenkins fix before merging the 'real' core change though, right? [02:58:21] yes [02:58:31] Hmmph. [03:00:38] legoktm[SJS]: Merge https://gerrit.wikimedia.org/r/#/c/424874/ and https://gerrit.wikimedia.org/r/#/c/424873/ so we can close out Remex now? [03:00:45] (Hopefully.) [03:00:55] argh sorry, I keep getting distracted every time you ask me :p [03:01:34] :-D [03:01:42] It's fine, the PHPU6 is rather bigger. [03:12:21] legoktm[SJS]: Umm. PSR2.Namespaces.NamespaceDeclaration.BlankLineAfter fails on 425942... whut? [03:12:32] I missed a newline [03:12:36] it's fixed in ps3 [03:12:40] but those test results are from ps2 [03:12:44] Oh, right. :-) [03:12:48] I was reading PS3 and was confused. [03:13:09] It'd be lovely if gerrit grouped comments by PS version. [03:16:31] Ha, the MF force-merged broke Krinkle's merge of your HamcrestPHPUnitIntegration patch. [03:16:44] I'll re-submit once jenkins replies. [03:17:17] ty [03:17:21] * Krinkle raises in high pitch voice: Did someone way force-merge? [03:17:27] s/way/say/ [03:17:42] yes :( circular dependency in tests [03:18:06] I thought Zuul supported those now when +2'ing around the same window with a mutual Depends-On. [03:18:18] But maybe not [03:18:37] If it changed I'm not aware of it [03:19:31] Krinkle: If two-way Depends-Ons work now I'll be a very happy bunny. [03:19:39] Krinkle: But if so an announcement would have been nice. :-) [03:19:53] k, nevermind me then [03:20:33] Krinkle: Do you know how to patch and release a new version of Jenkins? [03:20:50] I do not. [03:20:54] Meh. [03:20:56] What needs patching? [03:21:04] Like, jenkins.war? [03:27:14] One of the plugins. [03:27:24] Krinkle: https://phabricator.wikimedia.org/T192120 [03:32:21] You deploy the plugins :) [03:32:26] Should be easy [03:33:19] Though requires I think the plugins built hpi file [03:33:52] https://jenkins.io/doc/book/managing/plugins/ [03:34:00] legoktm[SJS]: James_F ^^ [03:34:38] I'm not really sure how we've installed them in the past [03:34:42] I was just going to wait for hashar [03:37:41] * James_F is just impatient. :-) [03:38:07] Well, that but also if we break the world I'd like as much time as possible before the cut on Tuesday for us to fix it, rather than being in a mess. [03:39:33] If hashar isn't able to fix it tomorrow I think we can just disable the xUnit plugin [03:39:47] (for that job) [03:40:03] legoktm[SJS]: I'm going to merge https://gerrit.wikimedia.org/r/#/c/425883/ (Wikidata fix-up) now unless you say no. [03:40:23] that one is ready [03:41:13] "OK, but incomplete, skipped, or risky tests!" from both mediawiki-extensions-php70-jessie and mediawiki-phpunit-php70-jessie [03:42:56] For that matter, the Wikidata WIP https://gerrit.wikimedia.org/r/#/c/425942/ passes (except for the MF force-merge artefact and 8 risky tests). [03:44:15] I'm fixing the last 8 now [03:46:13] Nice. [03:46:31] I just pushed a new title, sorry. [03:50:06] Of course, once this all lands there's the "fun" decision of whether we're going to follow the RfC and make REL1_31 require PHP7 (with lots of quick CI fixes for that branch needed) whilst master is still 5.* compatible for a bit, or ignore the RfC and make us miserable for four years. [03:51:00] Are we not ready for master yet? [03:51:28] Not in production, I don't think? [03:51:31] Maybe we are? [03:52:35] Certainly CI will immediately break because the script setting up MW for qunit runs on the php5 binary. [03:53:38] https://phabricator.wikimedia.org/T190548 [03:54:01] Aha, I did file a task. Yes. [03:54:44] worst case we can make REL1_31 require PHP 7 without any of the cool PHP 7 features [03:55:14] Yeah. But having master be PHP5.5 with REL1_31 being PHP7 will be a little bit of a pain. [03:55:25] So if at all possible let's not. :-) [03:57:58] https://integration.wikimedia.org/ci/job/mediawiki-extensions-php70-jessie/1313/console woo all good! [03:58:52] Brilliant. [04:00:28] so the wikibase hamcrest one can be merged now [06:52:18] anomie: https://phabricator.wikimedia.org/T191863#4129069 bisected for you ;) [14:45:00] https://github.com/phpstan/phpstan anyone? [15:22:09] Reedy: Isn't that what phan is supposed to do for us? Except "phpstan" sounds like the name of a country somewhere [15:33:27] lol [15:33:36] branch testing [15:33:40] infection testing [15:34:56] bd808: Meeting? [16:59:08] anomie: is https://phabricator.wikimedia.org/T191863#4129069 safe to revert? or would you like to try fixing it first? [17:00:04] legoktm: I'd rather not revert it, I spent all morning figuring out WTF is actually going on there. [17:00:26] ok :p [17:07:06] legoktm: In case you're curious, https://phabricator.wikimedia.org/T191863#4130112 explains it. Now I'll figure out a patch. [17:07:36] It didn't help that the test in question doesn't result in any logs by default... [17:08:43] ah [17:08:56] if you need logs/reproduction I can help with that [17:09:16] (if you have docker installed you can also try using quibble yourself :)) [17:13:07] I had to move my LocalSettings.php out of the way and reinstall using sqlite, but now I can reproduce it. [17:33:55] James_F: if the horizon/CI downtime is over, let's land PHPUnit 6 now? [17:34:28] horizon/ci downtime is over now [17:34:36] horizion has been put out of read only mode [17:34:51] https://gerrit.wikimedia.org/r/#/c/426113/ [17:37:00] Sure. [17:45:56] legoktm: https://gerrit.wikimedia.org/r/c/426120/ [17:55:18] anomie: +1'd, I'll let AaronSchulz take a look otherwise I'll merge it by the end of the day [18:23:42] legoktm: Is there a task for "Drop PHPUnit 4.x entirely"? Presumably T177132 is just "allow both 6 and 4"? [18:23:42] T177132: Run MediaWiki tests with PHPUnit 6 - https://phabricator.wikimedia.org/T177132 [18:24:00] whenever we drop PHP < 7.0 we can drop it [18:24:08] er [18:24:20] well whenever we drop HHVM that pretends to be less than PHP 7 [18:27:22] Right. The HHVM that now runs in CI fails (https://gerrit.wikimedia.org/r/#/c/426058/ -> https://integration.wikimedia.org/ci/job/mediawiki-extensions-hhvm-jessie/42766/console "phpunit 6.5.8 requires php ^7.0 -> your HHVM version does not satisfy that requirement"). [18:27:47] So maybe blocked by "Drop HHVM compat." blocked by "Migrate WMF prod to PHP7"? [18:27:50] * James_F makes tasks [18:32:07] Filed as T192167 and T192166. [18:32:08] T192166: Drop HHVM support from MediaWiki - https://phabricator.wikimedia.org/T192166 [18:32:08] T192167: Drop support for PHP Unit 4.x - https://phabricator.wikimedia.org/T192167 [19:01:58] James_F: libraryupgrader is running, it's just waiting for the zuul queue to be emptier before submitting more patches [19:02:11] * James_F nods. [19:02:20] Lots of fails? [19:02:41] it's still doing the canaries [19:02:49] it's only run against E:Linter so far heh [19:03:26] If that failed I'd be shocked given the author. ;-) [20:34:08] legoktm: We could presumably drop 4.x earlier if composer is fixed (or monkey patched) to understand hhvm versions [20:34:17] The same way MW does [20:34:31] PHPUnit 6 itself runs fine on HHVM [20:34:39] Or --ignore-platform-reqs [20:34:42] on the hhvm job [20:34:50] That'd be easiest probably [20:35:05] James_F: [20:37:22] I think --ignore-platform-reqs is too aggressive in that it ignores everything [20:37:44] No rush. [20:38:30] legoktm: Well, for unit tests on hhvm, assuming there is a separate php7 job, that seems perfectly fine to me. [20:38:38] If something does depend on something, we'll find out at run-time. [20:39:00] or from the php7 job [20:46:17] legoktm: I should have mentioned my suspicious about doRollback() having no trx where as service side rollback goes back to BEGIN. I didn't look into that anymore though. [20:48:33] AaronSchulz: Quick merge on that very much appreciated so we can switch the coverage job over from PHP5 to PHP7 in a known-good state. No pressure. :-) [20:53:39] yeah, looking (and doing some labs puppet thing at the same time) [21:07:20] James_F, anomie: I added a comment (not a blocker though) [21:13:40] AaronSchulz: Replied [21:17:45] anomie: right. That was the last patch before trxStatus. [21:18:01] another thing on my todo list then [21:19:24] huh, it *was* in that trxStatus one [21:21:07] https://gerrit.wikimedia.org/r/#/c/420718/10/includes/libs/rdbms/database/Database.php [21:21:39] nevermind, I had that right before. It was the connection cleanup patch before the trxStatus one. [21:29:30] it should have have doBegin() then, though error handling model at that time was already fairly broken. I removed the doRollback() in https://gerrit.wikimedia.org/r/#/c/421496/15/includes/libs/rdbms/database/Database.php . I should have made that an ERROR case rather than OK (which failed to account for postgres being stuck in error state unlike mysql or such). [21:32:23] https://gerrit.wikimedia.org/r/#/c/424375/1/includes/libs/rdbms/database/Database.php should have added the doBegin() or switched to ERROR. [21:34:02] heh, every patch to that spot keeps getting it wrong ;) [21:36:11] that only caveat to rollback+begin is that a view snapshot might be silently discarded [22:00:17] legoktm: suggestions re https://gerrit.wikimedia.org/r/#/c/405974/ ? just put it into Shell? [22:01:29] testing with two mysql clients deadlocking shows that the snapshot defined by SELECT is preserved on deadlock (rolls back to BEGIN). If we keep the automatic rollback() code then that won't hold true for usage of IDatabase. Thakes me lean against rollback (and also rollback+begin) a bit and lean more towards just using ERROR state. [22:05:03] I see that canRecoverFromDisconnect() ignores snapshot loss from non-explicit transactions with no atomic sections active...I suppose that is reasonable. If we are consistent with that then such cases of !wasKnownStatementRollbackError() would do rollback+begin and others would set ERROR. [22:05:40] anomie, James_F: anyway, if really want this for CI I can just +2 it [22:05:45] *if you [22:13:59] MaxSem: Shell seems like a good choice, unless we create a new class or something [22:14:02] AaronSchulz: yes please [22:28:00] done [22:30:51] _joe_: feel to merge it now