Page MenuHomePhabricator
Create Task
Maniphest T239642

We need to run tests on the PHP code in the deploy repo
Closed, ResolvedPublic
Actions

Description

We caused a production fault because our PHP vendor directory was missing some files. :(

Details

SubjectRepoBranchLines +/-
More fixes to make composer test workmediawiki/services/parsoid/deploymaster+15 K -24
layout: [parsoid/deploy] Make PHP tests votingintegration/configmaster+1 -4
Maintenance: Point in a different direction so this passes in /deploy if neededmediawiki/services/parsoidmaster+6 -1
Provide a composer test entry point for pre-prod final verificationmediawiki/services/parsoid/deploymaster+75 -2 K
layout: [parsoid/deploy] Add parsoidsvc-deploy-composer-testonly-php72-dockerintegration/configmaster+1 -1
[jjb] Provide parsoidsvc-deploy-composer-php72-docker to testintegration/configmaster+29 -0
layout: [Parsoid deploy] Add PHP72 tests as experimentalintegration/configmaster+2 -0
Customize query in gerrit

Related Objects

Event Timeline

cscott created this task.Dec 2 2019, 6:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 2 2019, 6:00 PM
Arlolra claimed this task.Dec 2 2019, 7:19 PM
• ssastry moved this task from Backlog to Testing / QA on the Parsoid-PHP board.Dec 2 2019, 7:27 PM
gerritbot added a comment.Dec 2 2019, 8:17 PM

Change 554140 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: [Parsoid deploy] Add PHP72 tests as experimental

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

gerritbot added a project: Patch-For-Review.Dec 2 2019, 8:17 PM
gerritbot added a comment.Dec 2 2019, 8:22 PM

Change 554140 merged by jenkins-bot:
[integration/config@master] layout: [Parsoid deploy] Add PHP72 tests as experimental

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

Jdforrester-WMF subscribed.Dec 2 2019, 8:30 PM

I've quickly added the parsoidsvc-composer-package-php72-docker job as experimental, but that's insufficient because (a) using the sub-repo doesn't quite work, and (b) that's a remote-fetch composer job which won't test the particular issue in question here.

Legoktm subscribed.Dec 2 2019, 8:46 PM

IIRC we should be able to use a job like mediawiki/vendor which uses the existing dependencies, and adds the development dependencies on top. I forgot how exactly that's implemented now and whether it's quibble specific.

Maintenance_bot removed a project: Patch-For-Review.Dec 2 2019, 9:10 PM
Arlolra removed Arlolra as the assignee of this task.Dec 2 2019, 10:04 PM
Arlolra subscribed.
Arlolra mentioned this in T242782: Stop running Parsoid-JS tests in CI.Jan 14 2020, 7:15 PM
Jdforrester-WMF triaged this task as High priority.Jan 14 2020, 7:15 PM
Jdforrester-WMF added a project: Release-Engineering-Team-TODO (2020-01 to 2020-03 (Q3)).

Oh, totally forgot about this. Not good. Nearing on UBN.

gerritbot added a comment.Jan 14 2020, 7:36 PM

Change 564764 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/services/parsoid/deploy@master] Provide a composer test entry point for pre-prod final verification

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

gerritbot added a project: Patch-For-Review.Jan 14 2020, 7:36 PM
gerritbot added a comment.Jan 14 2020, 7:51 PM

Change 564767 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] [jjb] Provide parsoidsvc-deploy-composer-php72-docker to test

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

gerritbot added a comment.Jan 14 2020, 7:51 PM

Change 564768 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: [parsoid/deploy] Wire up parsoidsvc-deploy-composer-php72-docker as experimental

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

gerritbot added a comment.Jan 14 2020, 7:51 PM

Change 564769 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: [parsoid/deploy] Make parsoidsvc-deploy-composer-php72-docker voting

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

gerritbot added a comment.Jan 14 2020, 8:26 PM

Change 564767 merged by jenkins-bot:
[integration/config@master] [jjb] Provide parsoidsvc-deploy-composer-php72-docker to test

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

gerritbot added a comment.Jan 14 2020, 8:34 PM

Change 564768 merged by jenkins-bot:
[integration/config@master] layout: [parsoid/deploy] Add parsoidsvc-deploy-composer-testonly-php72-docker

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

gerritbot added a comment.Jan 14 2020, 8:38 PM

Change 564764 merged by jenkins-bot:
[mediawiki/services/parsoid/deploy@master] Provide a composer test entry point for pre-prod final verification

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

Jdforrester-WMF added a comment.Jan 14 2020, 9:32 PM

So, we got so close on this, but it doesn't work.

composer test in parsoid-deploy runs parsoid's bin/toolcheck.php.sh, which loads parse.php, which loads Maintenance.php, which tries to load parsoid's autoload.php from its vendor directory (which is empty in the parsoid-deploy sub-module).

Manually hacking the Maintenance.php file to use parsoid-deploy's vendor directory then throws PHP Fatal error: Uncaught Error: Class 'Monolog\Logger' not found in /src/src/src/Config/Api/SiteConfig.php:117…

Arlolra added a comment.Jan 14 2020, 9:49 PM
diff --git a/composer.json b/composer.json
index 3faae9a20..df6195d45 100644
--- a/composer.json
+++ b/composer.json
@@ -29,6 +29,7 @@
                "wikimedia/scoped-callback": "3.0.0",
                "wikimedia/wikipeg": "2.0.3",
                "wikimedia/zest-css": "1.1.2",
+               "monolog/monolog": "~1.25.2",
                "ext-dom": "*",
                "ext-json": "*"
        },
@@ -36,7 +37,6 @@
                "jakub-onderka/php-parallel-lint": "1.0.0",
                "jakub-onderka/php-console-highlighter": "0.4.0",
                "jakub-onderka/php-console-color": "0.2",
-               "monolog/monolog": "~1.25.2",
                "mediawiki/mediawiki-codesniffer": "28.0.0",
                "mediawiki/mediawiki-phan-config": "0.9.0",
                "mediawiki/minus-x": "0.3.2",
@@ -49,14 +49,14 @@
        },
        "autoload": {
                "psr-4": {
-                       "Parsoid\\": ["src/placeholder", "src"]
+                       "Parsoid\\": ["src/placeholder", "src"],
+                       "Parsoid\\Tools\\": "tools/",
+                       "Parsoid\\Tests\\": "tests/mocks/"
                }
        },
        "autoload-dev": {
                "psr-4": {
-                       "Parsoid\\Tests\\": "tests/mocks/",
                        "Parsoid\\Tests\\ParserTests\\": "tests/ParserTests",
-                       "Parsoid\\Tools\\": "tools/",
                        "Test\\": "tests/phpunit/"
                }
        },
Jdforrester-WMF claimed this task.Jan 15 2020, 5:32 PM
Jdforrester-WMF moved this task from INBOX to New Work on the Release-Engineering-Team-TODO (2020-01 to 2020-03 (Q3)) board.
Arlolra added a comment.Jan 15 2020, 6:32 PM

Here's a patch with the above diff,
https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/deploy/+/564792

cscott added a comment.Jan 22 2020, 5:20 PM

Alternatively, could we just load monolog on-demand? I think there was an outstanding issue where we didn't actually want command-line invocations of parsoid to be logging to the production servers (or anywhere) by default. If we make no-logging (or log-to-stderr) the default command-line invocation, we wouldn't have to move monolog to prod dependencies. There should be no reason for mocks to be in the prod autoload, either. It would be nice if we could arrange things such that tools/ isn't needed: the entry point script can be in tools/ as long as it doesn't then depend on anything else in tools (at least not w/o manually adding that path to the autoload path).

Just trying to reduce our security footprint by keeping unnecessary code out of the prod autoload path. (Granted, production MW uses monolog itself, so that's not really opening up our security footprint, keeping monolog out is more about "no non-local logging by default" for CLI scripts.)

gerritbot added a comment.Feb 10 2020, 8:35 PM

Change 567159 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/services/parsoid@master] Maintenance: Point in a different direction so this passes in /deploy if needed

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

gerritbot added a comment.Feb 10 2020, 8:35 PM

Change 564815 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/services/parsoid/deploy@master] More fixes to make composer test work

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

gerritbot added a comment.Feb 10 2020, 11:16 PM

Change 567159 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Maintenance: Point in a different direction so this passes in /deploy if needed

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

gerritbot added a comment.Feb 10 2020, 11:42 PM

Change 564769 merged by jenkins-bot:
[integration/config@master] layout: [parsoid/deploy] Make PHP tests voting

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

Stashbot added a comment.Feb 10 2020, 11:42 PM

Mentioned in SAL (#wikimedia-releng) [2020-02-10T23:42:27Z] <James_F> Zuul: [parsoid/deploy] Make PHP tests voting T239642

gerritbot added a comment.Feb 10 2020, 11:58 PM

Change 564815 merged by jenkins-bot:
[mediawiki/services/parsoid/deploy@master] More fixes to make composer test work

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

Maintenance_bot removed a project: Patch-For-Review.Feb 11 2020, 12:10 AM
Jdforrester-WMF closed this task as Resolved.Feb 11 2020, 12:16 AM
Aklapper removed a subscriber: Parsing-Team--ARCHIVED.May 24 2023, 1:09 PM
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