Page MenuHomePhabricator
Create Task
Maniphest T319459

Re-work Minify's testRunJsFile test to not use forbidden @runInSeparateProcess which breaks CI publishing
Closed, ResolvedPublic
Actions

Description

Split out from T319458: Investigate potential issues with pcov in phpunit-coverage-php74-docker-publish.

In T319458#8288231, @Daimona wrote:

testRunJsFile uses the @runInSeparateProcess to, well, run in a separate process. I suspect that the CLI options for enabling pcov are not being passed through. This seems to be the only test in codesearch using the option, and codesniffer also forbids the annotation. I think the easiest thing would be to refactor the test to avoid the option.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
tests: Don't 'fix' coverage of ensureExpandedStates() via @runInSeparateProcessmediawiki/libs/Minifymaster+4 -7
Customize query in gerrit

Related Objects

Event Timeline

Jdforrester-WMF created this task.Oct 5 2022, 6:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2022, 6:50 PM
Jdforrester-WMF updated the task description. (Show Details)Oct 5 2022, 6:50 PM
Jdforrester-WMF added a subscriber: Daimona.
Daimona mentioned this in T319458: Investigate potential issues with pcov in phpunit-coverage-php74-docker-publish.Oct 5 2022, 6:52 PM
Jdforrester-WMF added a comment.Oct 5 2022, 6:54 PM

This annotation was brought in with a change to improve test coverage, and though the repo was building fine with the previous commit ever since then (2021-03-09) https://doc.wikimedia.org/cover/mediawiki-libs-Minify/ hasn't been updated.

gerritbot added a comment.Oct 5 2022, 6:55 PM

Change 838817 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/libs/Minify@master] Revert "tests: Fix code coverage of ensureExpandedStates() and address Doxygen warnings"

https://gerrit.wikimedia.org/r/838817

gerritbot added a project: Patch-For-Review.Oct 5 2022, 6:55 PM
Daimona added a subscriber: Krinkle.Oct 5 2022, 9:50 PM

If the goal is to reset $expandedStates after CliTest to its default value, what about an @internal resetExpandedStatesForTests method that could be called in setUp? It seems trivial to implement and should have the desired effect.

gerritbot added a comment.Oct 10 2022, 3:50 AM

Change 838817 merged by jenkins-bot:

[mediawiki/libs/Minify@master] tests: Don't 'fix' coverage of ensureExpandedStates() via @runInSeparateProcess

https://gerrit.wikimedia.org/r/838817

Maintenance_bot removed a project: Patch-For-Review.Oct 10 2022, 4:30 AM
Krinkle closed this task as Resolved.Oct 17 2022, 7:13 PM
Krinkle assigned this task to Jdforrester-WMF.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits