Change 547642 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] Revert "jjb: Bump all jobs for new php71/72/73 image cascade"

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

Change 547642 merged by jenkins-bot:
[integration/config@master] Revert "jjb: Bump all jobs for new php71/72/73 image cascade"

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

15:34:43  Failed asserting that two arrays are equal.
15:34:43  --- Expected
15:34:43  +++ Actual
15:34:43  @@ @@
15:34:43  -        'body' => '<tr><td colspan="2" class="diff-lineno" id="mw-diff-left-l1" >Line 1:</td>\n
15:34:43  -        <td colspan="2" class="diff-lineno">Line 1:</td></tr>\n
15:34:43  -        <tr><td class='diff-marker'>−</td><td class='diff-deletedline'><div><del class="diffchange diffchange-inline">A </del>4</div></td><td class='diff-marker'>+</td><td class='diff-addedline'><div><ins class="diffchange diffchange-inline">B </ins>4</div></td></tr>\n
15:34:43  +        'body' => '<tr>\n
15:34:43  +          <td colspan="2" class="diff-lineno">Line 1:</td>\n
15:34:43  +          <td colspan="2" class="diff-lineno">Line 1:</td>\n
15:34:43  +        </tr>\n
15:34:43  +        <tr>\n
15:34:43  +          <td class="diff-marker">−</td>\n
15:34:43  +          <td class="diff-deletedline"><div><del class="diffchange diffchange-inline">A</del> 4</div></td>\n
15:34:43  +          <td class="diff-marker">+</td>\n
15:34:43  +          <td class="diff-addedline"><div><ins class="diffchange diffchange-inline">B</ins> 4</div></td>\n
15:34:43  +        </tr>\n

The "expected" looks like what TableDiffFormatter.php would produce (e.g. header from here), while the "actual" looks like wikidiff2 instead (e.g. header from here).

Presumably $this->engine is evaluating to ENGINE_PHP (default) instead of ENGINE_WIKIDIFF2.

setEngine() is being called from getSlotDiffRendererInternal. wgExternalDiffEngine defaults to false. Is wikidiff2_do_diff somehow not showing up any more?

In T237049#5625101, @Jdforrester-WMF wrote:

Presumably $this->engine is evaluating to ENGINE_PHP (default) instead of ENGINE_WIKIDIFF2.

setEngine() is being called from getSlotDiffRendererInternal. wgExternalDiffEngine defaults to false. Is wikidiff2_do_diff somehow not showing up any more?

I think you have it backwards. The tests are expecting to be run without wikidiff2, but somehow the CI tests have started using it.

Aha, right, well, that makes sense because of 34c8b6e0 for T236333.

*sighs*

@Jdforrester-WMF Yeah, that was for our team to support integration tests against the MW REST API endpoints that use Wikidiff2, did we cause knock-on breakages?

In T237049#5632579, @WDoranWMF wrote:

@Jdforrester-WMF Yeah, that was for our team to support integration tests against the MW REST API endpoints that use Wikidiff2, did we cause knock-on breakages?

Yes. We can't update CI for any of MW core/extensions/skins right now. Will write a patch moving these tests in MW to a lower level.

Actually, no. I've spent three hours trying to disentangle this mess and I'm deep in the bowels of ContentHandler and its assumptions.

@daniel is probably needed to fix this, sorry.

In T237049#5632750, @Jdforrester-WMF wrote:

@daniel is probably needed to fix this, sorry.

I'm traveling/conferencing the next couple of weeks, so won't happen soon. Maybe @Tgr can give some hints.

$wgExternalDiffEngine defaults to false which also mean it would first use wikidiff2 when the function wikidiff2_do_diff is detected. There is no way to force the php native one :-\

Abstract:

    public static function getEngine() {
        $externalDiffEngine = MediaWikiServices::getInstance()->getMainConfig()
            ->get( 'ExternalDiffEngine' );

...

        if ( function_exists( 'wikidiff2_do_diff' ) ) {
            return 'wikidiff2';
        }

        // Native PHP
        return false;
    }

So that always return wikidiff2.

I guess we want to enhance the setting ExternalDiffEngine to allow php native. This way the couple tests failing can be forced to use the native php rendering.

The task description refers to the addition of the intl extension. That does not seem related. I guess it is because the CI image had wikidiff2 added but only the api-testing job had been switched to it?

In T237049#5633455, @hashar wrote:

The task description refers to the addition of the intl extension. That does not seem related. I guess it is because the CI image had wikidiff2 added but only the api-testing job had been switched to it?

Yup, pretty much. Adding intl (and James pushing it out) was the action that pushed also the wikidiff2 changes out to numerous other CI images. Which should've been done originally

Given other pressing needs, I'd recommend reverting the addition of wikidiff2 to the quibble images (which will mean either the API testing suite stops working, or we hold it back at the moderately-broken stage).

The traditional hack has been to set the diff engine to something like /dev/null, but rMW5b3bbd5adea3: Drop strings for wgExternalDiffEngine, deprecated in 1.27 and 1.32 broke that; IIUC the breakage did not manifest on that patch because the CI environment did not have wikidiff installed at the time.

So the simplest short-term fix would be just reverting that. Or maybe replacing with something less hacky where there is a special value you can pass (native or whatever) for using the PHP engine.

Change 548763 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Add $wgDiffEngine

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

Change 548763 merged by jenkins-bot:
[mediawiki/core@master] Add $wgDiffEngine

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

What's next? Set $wgDiffEngine in CI to PHP, and roll out the updated images again?

All tests that rely on a specific diffengine implementation should set it themselves (and from glancing at Brad's patch, they already do).

Change 549086 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] Re-apply "jjb: Bump all jobs for new php71/72/73 image cascade"

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

Change 549088 had a related patch set uploaded (by Jforrester; owner: Anomie):
[mediawiki/core@REL1_34] Add $wgDiffEngine

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

Change 549088 merged by jenkins-bot:
[mediawiki/core@REL1_34] Add $wgDiffEngine

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

Mentioned in SAL (#wikimedia-releng) [2019-11-06T15:40:31Z] <James_F> Upgraded mediawiki-quibble-composer-mysql-php72-docker as a test flight for T236907 T237049

Change 549086 merged by jenkins-bot:
[integration/config@master] Re-apply "jjb: Bump all jobs for new php71/72/73 image cascade"

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

Awesome, thank you for the backport to REL1_34!

Change 549670 had a related patch set uploaded (by Krinkle; owner: Anomie):
[mediawiki/core@wmf/1.35.0-wmf.5] Add $wgDiffEngine

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

Change 549670 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.5] Add $wgDiffEngine

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