[go: nahoru, domu]

Page MenuHomePhabricator

SummaryFormatterTest.php fails when run in a suite with ApiParseDiscussionToolsTest.php
Closed, ResolvedPublic

Description

Problem

Wikibase/repo/tests/phpunit/includes/SummaryFormatterTest.php has PHPUnit test failures when run in a suite with DiscussionTools/tests/phpunit/integration/ApiParseDiscussionToolsTest.php. 6 tests fail:

1) Wikibase\Repo\Tests\SummaryFormatterTest::testOnFormat with data set #4 ('item', 'wikibase-item', false, 'wbsetlabel-set:1|<>', false, 'Title', false, '!<span dir="auto"><span class...span>!')
Didn't find the expected final comment
Failed asserting that '‎<span dir="auto"><span class="autocomment">Changed [&lt;>] label</span></span>' matches PCRE pattern "!<span dir="auto"><span class="autocomment">.*?\[&#60;&#62;\].*?</span></span>!".

2) Wikibase\Repo\Tests\SummaryFormatterTest::testOnFormat with data set #5 ('item', 'wikibase-item', false, 'wbsetlabel-set:1|&lt;&gt;', false, 'Title', false, '!<span dir="auto"><span class...span>!')
Didn't find the expected final comment
Failed asserting that '‎<span dir="auto"><span class="autocomment">Changed [&lt;>] label</span></span>' matches PCRE pattern "!<span dir="auto"><span class="autocomment">.*?\[&#60;&#62;\].*?</span></span>!".


3) Wikibase\Repo\Tests\SummaryFormatterTest::testOnFormat with data set #6
...

4) Wikibase\Repo\Tests\SummaryFormatterTest::testOnFormat with data set #7
...

5) Wikibase\Repo\Tests\SummaryFormatterTest::testOnFormat with data set #9
...

6) Wikibase\Repo\Tests\SummaryFormatterTest::testOnFormat with data set #10
...

Steps to reproduce
In a Mediawiki checkout with the Wikibase extension installed.

  1. Copy phpunit.dist.xml to phpunit.xml
  2. Add a test suite with the following two tests:
<testsuite name="failing_group">
  <file>extensions/DiscussionTools/tests/phpunit/integration/ApiParseDiscussionToolsTest.php</file>
  <file>extensions/Wikibase/repo/tests/phpunit/includes/SummaryFormatterTest.php</file>
</testsuite>
  1. Run the named test suite:
mw docker mediawiki exec -- composer run phpunit:entrypoint -- --testsuite failing_group

Observed behaviour
The test run fails

Expected behaviour
The tests pass

Event Timeline

DiscussionTools registers an onParserAfterTidy hook handler. In its transformHtml method, a check of HookUtils::isAvailableForTitle( $title ) is made which, if true, calls CommentFormatter::addDiscussionTools( $html, $pout, $title ). The call:

		$html = XMLSerializer::serialize( $container, [ 'innerXML' => true, 'smartQuote' => false ] )['html'];

on line 424 of the CommentFormatter then converts escaped entities (e.g. &#60;&#62;) back into their raw characters (<>).

The call HookUtils::isAvailableForTitle( $title ) returns false when SummaryFormatterTest is run in isolation, but when run in a suite together with the ApiParseDiscussionToolsTest, it returns true because $wgTitle is set to a page that exists (as opposed to being null when the test runs in isolation). MessageCache::parse pulls in global $wgTitle on line 1492 and sets the page for the parse to this value of none is set (as appears to be the case for the test):

		if ( !$page ) {
			$logger = LoggerFactory::getInstance( 'GlobalTitleFail' );
			$logger->info(
				__METHOD__ . ' called with no title set.',
				[ 'exception' => new RuntimeException ]
			);
			$page = $wgTitle;
		}

My proposed fix is to reset $wgTitle to null in the setup for the MediaWikiLangTestCase in MediaWiki. This isn't the most satisfying, but I imagine ensuring that every other extension resets its global state after test execution, or that MessageCache doesn't rely on global state are more complicated fixes. I also wouldn't want to comment on whether DiscussionTools' unescaping of those entities in the right thing to do.

Change #1032718 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/core@master] Clear $wgTitle state before suite runs

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

Looking at logstash, this codepath is also used in production, so it seems like just removing the fallback here would also not be an option.

Change #1032718 merged by jenkins-bot:

[mediawiki/core@master] Clear $wgTitle state before suite runs

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