Page MenuHomePhabricator
Create Task
Maniphest T203694

Run ForeignResourceManager verification on MediaWiki core commits
Closed, ResolvedPublic
Actions

Description

Now we can assert that our libraries aren't screwed with, we should.

[Not really RL, but nowhere better to put it?]

Details

ProjectBranchLines +/-Subject
integration/configmaster+3 -2Zuul: [mediawiki/core] Switch standalone job to bespoke one
integration/configmaster+10 -1jjb: Provide bespoke PHPUnit standalone job for MediaWiki core
mediawiki/coreREL1_40+5 -1ResourceLoader: Move ForeignResourceStructureTest to integration/
mediawiki/coremaster+5 -1ResourceLoader: Move ForeignResourceStructureTest to integration/
mediawiki/coremaster+2 -2Revert typo fixes to ForeignResourceManager managed files
mediawiki/coremaster+1 -0ResourceLoader: Mark ForeignResourceStructureTest as @group Standalone
integration/configmaster+5 -0Zuul: [mediawiki/core] Run standalone jobs
mediawiki/coremaster+73 -30ResourceLoader: Add structure test for foreign-resources.yaml
mediawiki/coremaster+39 -22resources: Make foreign-resources.yaml pass its own 'verify' test
mediawiki/coremaster+54 -8resources: Add caching for faster runs and offline use
Customize query in gerrit

Related Objects
Search...

Event Timeline

Jdforrester-WMF created this task.Sep 6 2018, 4:57 PM
Restricted Application added a project: Performance-Team. · View Herald TranscriptSep 6 2018, 4:57 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Krinkle added a subscriber: Krinkle.Sep 6 2018, 5:06 PM

Yeah, sounds good to me!

  • The urls are expected to be immutable. The main value we want to get from verifying is to assert that the checksum in our YAML is correct for the given url, and that the extracted files from the remote resource match what was checked into our Git repo. It's less important to verify that the remote copy hasn't changed.
  • Without a cache, the job would likely be quite slow. Might be fine if it's a separate job in parallel, but not if adding to the main job.
  • Without a cache, the job would be more likely to fail due to random upstream/network issues.
  • Regarding cache, probably we can use Castor for this? Alternatively, some generic Squid/http proxy might also work.
Krinkle triaged this task as Medium priority.Sep 7 2018, 1:15 AM
Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.
Krinkle added a project: MediaWiki-Core-Tests.
Krinkle moved this task from Inbox to Checkers on the MediaWiki-Core-Tests board.
Legoktm added a subscriber: Legoktm.Sep 7 2018, 1:35 AM

Is there a reason this needs to be something separate and can't be a standard PHPUnit test?

Krinkle added a comment.EditedSep 7 2018, 2:09 AM
In T203694#4564943, @Legoktm wrote:

Is there a reason this needs to be something separate and can't be a standard PHPUnit test?

The "this" as the maintenance script, yes, because it's not (just) a test.

The "this" as the solution for this task to run the verify step in CI, no, that doesn't need to be separate. But, there are considerations that might motivate one to run it separately. Specifically:

In T203694#4563676, @Krinkle wrote:

[..]

  • Without a cache, the job would likely be quite slow. [..]
  • Without a cache, the job would be more likely to fail due to random upstream/network issues.

But yes, if we do use a cache, and make it work fast, it should be fine to add to the "big" quibble job. But, if we can not make it fast, and/or if the cache is hard to get to work within PHPUnit, then it might make sense to run from a separate job in a way that just invokes the "verify" command from bash.

Krinkle moved this task from Inbox, needs triage to Backlog: Maintenance, non-prioritized on the Performance-Team board.Sep 10 2018, 6:02 PM
Krinkle moved this task from Backlog: Maintenance, non-prioritized to To-do: Goals, prioritized next 4 Quarters on the Performance-Team board.Sep 24 2018, 5:33 PM
gerritbot added a comment.Mar 24 2019, 8:48 PM

Change 498737 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resources: Add caching for faster runs and offline use

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

gerritbot added a project: Patch-For-Review.Mar 24 2019, 8:48 PM
gerritbot added a comment.Mar 25 2019, 8:13 PM

Change 498737 merged by jenkins-bot:
[mediawiki/core@master] resources: Add caching for faster runs and offline use

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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.23; 2019-03-26).Mar 25 2019, 9:00 PM
Krinkle removed projects: MW-1.33-notes (1.33.0-wmf.23; 2019-03-26), Patch-For-Review.EditedMar 26 2019, 12:42 AM

Actual Jenkins integration still to be done later.

Could be a PHPUnit structure test, or a separate job. Not sure yet.

Krinkle merged a task: T216619: Add the foreign resources integrity check to MediaWiki's unit tests.Apr 5 2019, 3:55 PM
Krinkle renamed this task from Run `maintenance/resources/manageForeignResources.php verify` as a test on MediaWiki core to Run ForeignResourceManager verification on MediaWiki core commits.Apr 5 2019, 3:57 PM
Jdforrester-WMF added a subtask: T216620: Add error detection to HTTP fetch in foreign resources checker before gzipping.Jun 21 2019, 10:15 PM
Krinkle mentioned this in T239246: Add curl support to mwext-node10-rundoc-docker.Dec 11 2019, 7:11 PM
Krinkle moved this task from To-do: Goals, prioritized next 4 Quarters to Abstract Wikipedia matrixing on the Performance-Team board.Apr 27 2021, 10:14 PM
Krinkle moved this task from Abstract Wikipedia matrixing to To-do: Goals, prioritized next 4 Quarters on the Performance-Team board.May 3 2021, 6:35 PM
Krinkle added a project: Developer Productivity.
hashar removed a project: Continuous-Integration-Config.Jun 4 2021, 10:20 AM
hashar added a subscriber: hashar.

Looks like that doesn't need any specific tweak on the CI side and it "just" need some extra tests in mediawiki/core. Hence dropping Continuous-Integration-Config in favor of MediaWiki-Core-Tests

gerritbot added a comment.Apr 3 2022, 5:08 PM

Change 776350 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Run "verify" action for foreign-resources.yaml in CI

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

gerritbot added a project: Patch-For-Review.Apr 3 2022, 5:08 PM
hashar removed a subscriber: hashar.Apr 20 2022, 12:38 PM
gerritbot added a comment.Aug 31 2022, 12:06 PM

Change 828509 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] ResourceLoader: Add structure test for foreign-resources.yaml

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

gerritbot added a comment.Aug 31 2022, 1:08 PM

Change 776350 merged by jenkins-bot:

[mediawiki/core@master] resources: Make foreign-resources.yaml pass its own 'verify' test

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.28; 2022-09-05).Aug 31 2022, 2:00 PM
Krinkle claimed this task.Oct 4 2022, 8:44 PM

Work in progress.

Krinkle lowered the priority of this task from Medium to Low.Oct 4 2022, 8:44 PM
Krinkle moved this task from To-do: Goals, prioritized next 4 Quarters to Backlog: Maintenance, non-prioritized on the Performance-Team board.
gerritbot added a comment.Oct 26 2022, 3:31 PM

Change 828509 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Add structure test for foreign-resources.yaml

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

Jdforrester-WMF closed this task as Resolved.Oct 26 2022, 3:35 PM
ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.8; 2022-10-31).Oct 26 2022, 4:00 PM
Maintenance_bot removed a project: Patch-For-Review.Oct 26 2022, 4:30 PM
hashar reopened this task as Open.Oct 27 2022, 12:23 PM
hashar added a project: ci-test-error.

When sending patch for Quibble we have a job running mediawiki/core tests. I had two build failures due to ForeignResourceStructureTest::testVerifyIntegrity doing HTTP requests to an external site:

There was 1 error:

1) ForeignResourceStructureTest::testVerifyIntegrity
Exception: Failed to download resource at https://codeload.github.com/wikimedia/jquery.i18n/tar.gz/70b5ee20a638cb8fe36baef8d51ac2eb577ce012

/workspace/src/includes/ForeignResourceManager.php:231
/workspace/src/includes/ForeignResourceManager.php:311
/workspace/src/includes/ForeignResourceManager.php:157
/workspace/src/tests/phpunit/structure/ForeignResourceStructureTest.php:33

ERRORS!
Tests: 26774, Assertions: 95085, Errors: 1, Skipped: 94.
There was 1 error:

1) ForeignResourceStructureTest::testVerifyIntegrity
Exception: Failed to download resource at https://raw.githubusercontent.com/briancherne/jquery-hoverIntent/823603fdac/jquery.hoverIntent.js

/workspace/src/includes/ForeignResourceManager.php:231
/workspace/src/includes/ForeignResourceManager.php:264
/workspace/src/includes/ForeignResourceManager.php:160
/workspace/src/tests/phpunit/structure/ForeignResourceStructureTest.php:33

ERRORS!
Tests: 26774, Assertions: 95085, Errors: 1, Skipped: 94.

The URLs come from resources/lib/foreign-resources.yaml and are checked by tests/phpunit/structure/ForeignResourceStructureTest.php which verifies them all. The structure test was introduced on October 26th by 3270283abf9f971722633ec88a2a467a5d955c37 for T203694

Maybe it could have some kind of caching to prevent CI from downloading all those URLs over and over again?

I think we should disable the test for now, looks like it download 39 resources on every single change passing through CI (since structure tests are run for everything).

Krinkle added a comment.Oct 27 2022, 1:52 PM

It has caching. Is castor working correctly for these jobs?

gerritbot added a comment.Oct 27 2022, 9:18 PM

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

[mediawiki/core@master] Follow-up 3270283abf: Mark ForeignResourceStructureTest as @group Standalone

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

gerritbot added a project: Patch-For-Review.Oct 27 2022, 9:18 PM
gerritbot added a comment.Oct 27 2022, 9:35 PM

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

[integration/config@master] Zuul: [mediawiki/core] Run standalone jobs

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

gerritbot added a comment.Oct 27 2022, 9:37 PM

Change 850151 merged by jenkins-bot:

[integration/config@master] Zuul: [mediawiki/core] Run standalone jobs

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

Stashbot added a comment.Oct 27 2022, 9:38 PM

Mentioned in SAL (#wikimedia-releng) [2022-10-27T21:38:48Z] <James_F> Zuul: [mediawiki/core] Run standalone jobs T203694

gerritbot added a comment.Oct 27 2022, 9:41 PM

Change 850276 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Mark ForeignResourceStructureTest as @group Standalone

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

Maintenance_bot removed a project: Patch-For-Review.Oct 27 2022, 10:31 PM
Krinkle closed this task as Resolved.Jan 19 2023, 7:36 AM
Krinkle edited projects, added Performance-Team-publish; removed ci-test-error.
Krinkle moved this task from Untriaged to Misc bookmarks on the Performance-Team-publish board.
Krinkle reopened this task as Open.Mar 28 2023, 11:55 PM
In T203694#8351385, @gerritbot wrote:

Change 850151 merged by jenkins-bot:

[integration/config@master] Zuul: [mediawiki/core] Run standalone jobs

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

It appears this job doesn't actually invoke phpunit. It runs install.php and the next step is shutting down MySQL and the job is marked as complete.

We encountered this today because PHPUnit is failing for everyone locally after https://gerrit.wikimedia.org/r/c/mediawiki/core/+/903778 which forgot to check in one of the new Codex files.

https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-phpunit-standalone-docker/5454/console : SUCCESS in 1m 23s

CI failed to catch it here because ForeignResourceStructureTest is marked Standalone, and the dediced job for that above appears not actually invoke phpunit.

gerritbot added a comment.Mar 29 2023, 12:00 AM

Change 903804 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Revert typo fixes to ForeignResourceManager managed files

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

gerritbot added a project: Patch-For-Review.Mar 29 2023, 12:00 AM
gerritbot added a comment.Mar 29 2023, 12:17 AM

Change 903804 merged by jenkins-bot:

[mediawiki/core@master] Revert typo fixes to ForeignResourceManager managed files

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

Maintenance_bot removed a project: Patch-For-Review.Mar 29 2023, 12:30 AM
ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.3; 2023-04-03).Mar 29 2023, 1:00 AM
gerritbot added a comment.Mar 29 2023, 11:18 AM

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

[integration/config@master] jjb: Provide bespoke PHPUnit standalone job for MediaWiki core

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

gerritbot added a project: Patch-For-Review.Mar 29 2023, 11:18 AM

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

[integration/config@master] Zuul: [mediawiki/core] Switch standalone job to bespoke one

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

gerritbot added a comment.Mar 29 2023, 9:17 PM

Change 904283 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] ResourceLoader: Move ForeignResourceStructureTest to integration/

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

gerritbot added a comment.Mar 30 2023, 7:21 AM

Change 904288 had a related patch set uploaded (by Hashar; author: Krinkle):

[mediawiki/core@REL1_40] ResourceLoader: Move ForeignResourceStructureTest to integration/

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

gerritbot added a comment.Mar 30 2023, 7:36 AM

Change 904283 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Move ForeignResourceStructureTest to integration/

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

gerritbot added a comment.Mar 31 2023, 1:21 AM

Change 904288 merged by jenkins-bot:

[mediawiki/core@REL1_40] ResourceLoader: Move ForeignResourceStructureTest to integration/

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

ReleaseTaggerBot edited projects, added MW-1.40-notes; removed MW-1.40-notes (1.40.0-wmf.8; 2022-10-31).Mar 31 2023, 4:00 AM
gerritbot added a comment.Mar 31 2023, 12:21 PM

Change 904100 abandoned by Hashar:

[integration/config@master] jjb: Provide bespoke PHPUnit standalone job for MediaWiki core

Reason:

After the test got moved out of @group Standalone by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/904283 I think this is no more needed.

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

gerritbot added a comment.Mar 31 2023, 12:21 PM

Change 904101 abandoned by Hashar:

[integration/config@master] Zuul: [mediawiki/core] Switch standalone job to bespoke one

Reason:

After the test got moved out of @group Standalone by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/904283 I think this is no more needed.

More details on the parent change which proposed jjb jobs: https://gerrit.wikimedia.org/r/c/integration/config/+/904100

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

Maintenance_bot removed a project: Patch-For-Review.Mar 31 2023, 12:30 PM
Krinkle closed this task as Resolved.Apr 3 2023, 5:49 AM
Jdforrester-WMF mentioned this in T274442: Test that files listed in the foreign-resources.yaml each exist in the repo, and pass the verify step.Apr 27 2023, 3:14 PM
Krinkle mentioned this in T330508: Expand running of ForeignResourceStructureTest against skins and extensions that have foreign-resources.yaml files.Apr 27 2023, 4:40 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