Page MenuHomePhabricator

JADE\Tests\Hooks\MoveHooksTest::testOnMovePageIsValidMove is broken
Closed, ResolvedPublic

Description

https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-hhvm-docker/27382/console

There was 1 failure:

1) JADE\Tests\Hooks\MoveHooksTest::testOnMovePageIsValidMove with data set #2 ('judgment-existing', 'main-new', array('content-not-allowed-here', 'JadeJudgment', 'main-new'))
Failed asserting that exception message '"JadeJudgment" content is not allowed on page [[:New page1653947107]] in slot "main"' contains '"JadeJudgment" content is not allowed on page [[:New page1653947107]] in slot "$3"'.

/workspace/src/tests/phpunit/MediaWikiTestCase.php:424
/workspace/src/maintenance/doMaintenance.php:94

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 8 2018, 2:42 PM
CCicalese_WMF added a subscriber: CCicalese_WMF.

This is likely caused by T194046. See also T211052, which was affected in the same way.

daniel added a comment.EditedDec 11 2018, 3:20 PM

The test case is now wrong. It's testing for an error message that used to be generated with a parameter missing ($3). The parameter is no correctly filled in (main). The assertion needs to be changed accordingly.

testing against full error messages is generally brittle. The wording of error messages is not a stable interface. You can set the language to qqx so you get the message raw.

awight added a subscriber: awight.EditedDec 14 2018, 12:53 AM

Thanks, this test case is fixed in commit I1fbdae09fbd00b300383a7c02a1c5a3ab5690c63 (I'll close this once it's merged).

testing against full error messages is generally brittle. The wording of error messages is not a stable interface. You can set the language to qqx so you get the message raw.

I think I'm doing something equivalent, it will only break when the structure of the parameters changes, the code looks like:

$expectedException = [ 'content-not-allowed-here', JudgmentContent::CONTENT_MODEL_JUDGMENT, self::MAIN_NEW, 'main' ];
$this->setExpectedApiException( $expectedException );
Harej moved this task from Inbox to Bugs on the Jade board.Dec 17 2018, 6:49 PM

The patch mentioned here is merged: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/JADE/+/476993
Should we call this done or is it still ongoing?

awight removed a subscriber: awight.Mar 21 2019, 4:05 PM
Halfak closed this task as Resolved.Apr 8 2019, 4:14 PM
Halfak claimed this task.
Halfak moved this task from Review to Done on the Scoring-platform-team (Current) board.