[00:19:09] @covers: "If provided, only the code coverage information for the specified method(s) will be considered." [00:21:18] so supposedly the PreprocessorTest tests only cover preprocessToXml(), not any of its helper functions [00:22:55] https://doc.wikimedia.org/cover/mediawiki-core/master/php/includes/parser/Preprocessor_DOM.php.html [00:25:43] yep [00:26:43] "@covers Preprocessor_DOM" at the top of the class should be good enough [00:27:56] why is it needed at all? [00:31:34] so tests that call code which calls Title::exists() or something doesn't show up as covering Title::exists() [00:35:03] looks like I could spend all day just broadening @covers directives [00:35:18] https://doc.wikimedia.org/cover/mediawiki-core/master/php/includes/libs/composer/ComposerJson.php.html [00:37:26] What about that class? [00:37:44] the constructor is supposedly uncovered [00:40:02] there aren't any explicit tests for it [00:41:14] yeah, maybe I just don't quite have the same level of idle ambition [00:42:20] which is why I suggested to just put a @covers tag for the entire class at the top of the test case [00:42:46] you were anticipating? [00:44:06] meh [00:44:34] I definitely do not want to open a policy discussion, that would drive me crazy [00:46:49] explict @covers tags are needed for tests that are basically integration tests so it doesn't mark random code as covered even though it's not actually covered by tests specific to it. for code that is properly isolated and we can write real unit tests for, it doesn't make much sense. so the class level tags are a good compromise [01:10:09] see what you think of https://gerrit.wikimedia.org/r/#/c/306858/ [01:11:47] I think some people are reading @covers as documentation, not as a code coverage filter [01:12:22] I included PPFrame_DOM in @covers there because although it's not tested currently, presumably PreprocessorTest is where the test will go when it is written [01:12:37] which makes sense if you read @covers as a coverage filter, right? [01:14:22] I get that different people have different ideas of how finely grained your tests have to be [01:15:49] and, absent a policy, the developer largely gets to choose both their aspirations and their actual achievements for code coverage [01:16:31] +2'd [01:17:02] and yeah, that makes sense to me [01:17:19] and it would be stepping on the toes of the original developer if integration tests leak coverage into areas which the developer aspires to cover separately [01:19:46] where it gets murky is when the developer's aspirations are unclear, which I think was the case in that ComposerJson test [01:20:16] maybe probably the constructor was uncovered because the developer thought it was sufficiently covered by the other tests, and has no intention of writing a separate test for it [01:20:46] in which case the @covers was mistaken [01:21:28] or maybe the @covers was intentional and the developer really is hoping someone will write a constructor test [01:21:33] As the author of the ComposerJson test, I have no idea what I was thinking ~2 years ago :P [01:21:48] heh [14:12:02] AaronSchulz: i have a job on which maintenance/runJobs.php fails silently. no error, no nothing, the job is not done. how do i begin debugging this? the job is an AssembleUploadChunks one [14:12:14] i already tried $wgDebugLog (nothing) [14:12:47] (i can consistently reproduce with the specific file) [14:17:14] hmm, it dies with error code 255 [15:09:48] bleh, it seems to be just "Allowed memory size of 157286400 bytes exhausted. i had display_errors off in the CLI php.ini file. :/ [15:23:30] (running with --memory-limit=max helps) [17:13:46] MatmaRex: maybe its the metadata parsing. What kind of file? [17:14:33] the actual backend parts for FS backend stream copy from chunk to the whole on disk [17:17:04] AaronSchulz: it probably was the metadata, it was a PDF. https://phabricator.wikimedia.org/T143993 [17:17:46] do we do those as badly as djvu? [17:17:47] AaronSchulz: --memory-limit=max helped, and after i bumped the RAM of the virtual machine to 4 GB it did not go into swap [17:17:51] AaronSchulz: yes, worse [17:17:59] that's possible? [17:18:35] i don't think we read the whole text of the DJVU, possibly multiple megabytes, and try to stick it into img_metadata? we do for PDF [17:18:58] AaronSchulz: anyway, i got through that, but the job was still taking more than fifteen minutes to run with cpu at 100%, so i just gave up [17:20:33] (it takes around a minute to do it for that file on Commons)