CI failing on wmf/1.41.0-wmf.20 due to Parsoid\Config\SiteConfigTest
Closed, ResolvedPublic
Actions

Assigned To

None

Authored By

	Krinkle
	Aug 11 2023, 3:24 AM

Description

Patches for the current production branch are consistently (e.g. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/947913) due to:

There were 11 errors:

1) MediaWiki\Tests\Unit\Parser\Parsoid\Config\SiteConfigTest::testLangConverterEnabled_exception
Error: Call to undefined method MediaWiki\Parser\Parsoid\Config\SiteConfig::langConverterEnabled()

/workspace/src/tests/phpunit/unit/includes/parser/Parsoid/Config/SiteConfigTest.php:662
/workspace/src/tests/phpunit/MediaWikiUnitTestCase.php:115
phpvfscomposer:///workspace/src/vendor/phpunit/phpunit/phpunit:97

It's not clear how this situation was created since we run CI on every patch and integrate in both directions. I suspect either CI was bypassed somewhere, or perhaps a race condition during the weekly branch creation where a combination of two critical patches in mediawiki-vendor and mediawiki-core ended up on different sides of the branch cut?

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/943612

- $this->assertFalse( $config->langConverterEnabled(
+ $this->assertFalse( $config->langConverterEnabledBcp47

https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/899093

- public function langConverterEnabled( string $lang ): bool {

/cc @cscott @ihurbain

Details

	Subject	Repo	Branch	Lines +/-
	Downgrade Parsoid in wmf.20	mediawiki/core	wmf/1.41.0-wmf.20	+1 -1
	tests: Temporarily disable automatic running of Wdio tests in CI	mediawiki/extensions/Wikibase	wmf/1.41.0-wmf.20	+1 -1

Customize query in gerrit

Related Objects

Mentioned In: T287419: `mediawiki-core-php72-phan-docker` job runs `composer install` instead of using packages from mediawiki/vendor
Mentioned Here: T287419: `mediawiki-core-php72-phan-docker` job runs `composer install` instead of using packages from mediawiki/vendor

Event Timeline

Krinkle created this task.Aug 11 2023, 3:24 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 11 2023, 3:25 AM

Krinkle triaged this task as Unbreak Now! priority.Aug 11 2023, 3:25 AM

In mediawiki/vendor branch wmf/1.41.0-wmf.20 I see Parsoid 0.18.0-a20 with the old methods still present in wikimedia/parsoid/src/Config/SiteConfig.php. Core .gitmodules in branch wmf/1.41.0-wmf.20 apparently points to the correct branch of mediawiki/vendor and has the right version in composer.json. So how does CI end up with Parsoid 0.18.0-a21?

composer.json has:

"wikimedia/parsoid": "^0.18.0-a1@alpha",

So it installs the latest which is a21.

Since it's a breaking change, if it was following semver, maybe a21 should have been 1.0.0.

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

[mediawiki/core@wmf/1.41.0-wmf.20] Downgrade Parsoid in wmf.20

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

gerritbot added a project: Patch-For-Review.Aug 11 2023, 4:53 AM

The vendor-jobs passed as that jobs using the version of parsoid in mediawiki/vendor. The composer-job failed as that installed via composer.json and using the newest version (which are allowed by the given version constraint).

The failure fixes should be in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/946625, which was used to unbreak CI in the master branch.

Parsoid seems not use semver right now, so pinning the version would help for master and wmf branches when bringing out breaking changes, but makes the release process complicated.
The parsoid team was not aware about the breaking change behaviour of this new release (but was also not the first time that CI breaks - last time only phan breaks: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/943634).

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

[mediawiki/extensions/Wikibase@wmf/1.41.0-wmf.20] tests: Temporarily disable automatic running of Wdio tests in CI

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

Change 948109 merged by Krinkle:

[mediawiki/extensions/Wikibase@wmf/1.41.0-wmf.20] tests: Temporarily disable automatic running of Wdio tests in CI

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

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.20; 2023-08-01).Aug 11 2023, 2:00 PM

Do any of you know why it took till end of the week for this to break CI given that Parsoid's -a21 was tagged on Monday?

Ok, let me try to see what's going on. Parsoid tagged -a21 and released it to mediawiki-vendor on Monday (https://gerrit.wikimedia.org/r/c/mediawiki/vendor/+/946615), well before the wmf.20 branch I believe. There's a "well known" bug in CI that means certain jobs don't run from mediawiki-vendor: T287419: `mediawiki-core-php72-phan-docker` job runs `composer install` instead of using packages from mediawiki/vendor. As a result, we broke CI for some period between when we tagged -a21 and when we actually merged it into mediawiki-vendor. Merging into mediawiki-vendor was "slow" (took a couple of hours) because there were some leftover dependencies in core on the old version, which the gate-and-submit for mediawiki-vendor correctly discovered. Once the mediawiki-vendor job merged, there weren't any further CI issues, AFAIK.

It *seems* to me that the issue with wmf.20 is the same as that flagged in T287419; that is, some CI tasks (not just on master, but for branched revisions) are using composer install instead of using the appropriately versioned mediawiki-vendor. To answer subbu's question, this "only occurred" because the train was stalled (i'm guessing) and we were still trying to backport stuff to wmf.20 even after parsoid had tagged and released for wmf.21. Basically once Parsoid tags, T287419 takes over.

(This isn't an issue with "semver"; Parsoid uses an alpha version specifically because our weekly train rolls aren't "releases" with compatibility guarantees. Using an alpha version in the composer.json in core was a compromise worked out with some difficulty years ago to avoid having to do an additional commit to core every week to bump the version tag.)

I think this is a dup of T287419 and we should just finally fix that bug.

In T344032#9086445, @Umherirrender wrote:

Parsoid seems not use semver right now, so pinning the version would help for master and wmf branches when bringing out breaking changes, but makes the release process complicated.

We should just fix T287419. mediawiki-vendor is the mechanism for pinning versions, and it works well.

The parsoid team was not aware about the breaking change behaviour of this new release (but was also not the first time that CI breaks - last time only phan breaks: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/943634).

We actually battle quite gnarly circular dependencies between core and Parsoid on a somewhat regular basis. It's pretty hard to do active development on a tightly-coupled library like Parsoid unless you can introduce a new feature in Parsoid and then use it in core. We do that by tagging Parsoid and releasing to mediawiki-vendor, and then having the core change depend on the new version in mediawiki-vendor. This works even without merging the mediawiki-vendor patch (except for T287419) and even without tagging, if we're clever enough to use the appropriate gerrit git reference of a to-be-tested parsoid version in composer.json for a speculative release (except for T287419). Really, the only problem is that there are CI tasks which don't use the pinned version in mediawiki-vendor (aka T287419).

cscott mentioned this in T287419: `mediawiki-core-php72-phan-docker` job runs `composer install` instead of using packages from mediawiki/vendor.Aug 11 2023, 3:14 PM

Change 947989 merged by jenkins-bot: