Page MenuHomePhabricator

DonationInterface CI tests fail due to core change
Closed, ResolvedPublic1 Story Points

Description

Steps to reproduce

  • If you go here

https://integration.wikimedia.org/ci/job/mwext-DonationInterface-testextension-zend/2179/console

and

https://integration.wikimedia.org/ci/job/mwext-DonationInterface-testextension-zend/2178/console

Actual results

  • It keeps failing at

20:04:05 There were 2 failures:
20:04:05
20:04:05 1) DonationInterface_Adapter_GatewayAdapterTest::testResetOnRecurringSwitch
20:04:05 Order ID was not regenerated on recurring switch!
20:04:05 Failed asserting that '3047009065' is not equal to <string:3047009065>.
20:04:05
20:04:05 /mnt/jenkins-workspace/workspace/mwext-DonationInterface-testextension-zend/src/extensions/DonationInterface/tests/Adapter/GatewayAdapterTest.php:182
20:04:05 /mnt/jenkins-workspace/workspace/mwext-DonationInterface-testextension-zend/src/tests/phpunit/MediaWikiTestCase.php:137
20:04:05 /mnt/jenkins-workspace/workspace/mwext-DonationInterface-testextension-zend/src/tests/phpunit/phpunit.php:285
20:04:05
20:04:05 2) DonationInterface_Adapter_GatewayAdapterTest::testResetSubmethodOnMethodSwitch
20:04:05 Submethod was not blanked on method switch
20:04:05 Failed asserting that two strings are equal.
20:04:05 --- Expected
20:04:05 +++ Actual
20:04:05 @@ @@
20:04:05 -''
20:04:05 +'itau'
20:04:05
20:04:05 /mnt/jenkins-workspace/workspace/mwext-DonationInterface-testextension-zend/src/extensions/DonationInterface/tests/Adapter/GatewayAdapterTest.php:209
20:04:05 /mnt/jenkins-workspace/workspace/mwext-DonationInterface-testextension-zend/src/tests/phpunit/MediaWikiTestCase.php:137
20:04:05 /mnt/jenkins-workspace/workspace/mwext-DonationInterface-testextension-zend/src/tests/phpunit/phpunit.php:285

Expected results

  • The test should succeed.

Please fix the test.

Event Timeline

Paladox created this task.Jan 10 2016, 8:08 PM
Paladox updated the task description. (Show Details)
Paladox raised the priority of this task from to Needs Triage.
Paladox added a subscriber: Paladox.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptJan 10 2016, 8:08 PM
Paladox added a subscriber: hashar.Jan 11 2016, 1:04 PM

@hashar why is the test failing. Looking at the history no patches since December 23 or 24 were merged that would caused the test to break. Maybe it is because of that problem we had starting December 24 when qunit tests broke but the patches that were merged they merged ok. But now when you do recheck they error out with the error above as in the description above. How can we fix this.

Paladox removed a subscriber: hashar.Jan 11 2016, 1:04 PM

CCing MediaWiki reviewers and DonationInterface authors

The DonationInterface tests fail due to a change made to MediaWiki 4555d1b482816730b25e6956444ca6a3155dd2cd https://gerrit.wikimedia.org/r/#/c/253651/

$ git bisect start 4542566 04fdc78
$ git bisect run bash -c 'cd tests/phpunit; php phpunit.php --stop-on-error --stop-on-failure --testsuite extensions'
4555d1b482816730b25e6956444ca6a3155dd2cd is the first bad commit
commit 4555d1b482816730b25e6956444ca6a3155dd2cd
Author: Florian <florian.schmidt.stargatewissen@gmail.com>
Date:   Tue Nov 17 19:43:47 2015 +0100

    RequestContext: Load the request object for getRequest on first call
    
    Instead of relying on the global $wgRequest, which probably isn't initialized
    so far, create the request object when RequestContext::getRequest() is called
    the first time.
    
    Change-Id: I6115ba44e474619d02d456a103758fe73ed298e0

Though there is an unrelated failure, I have confirmed that git revert 4555d1b482816730b25e6956444ca6a3155dd2cd on core let the DonationInterface / ContributionTracking changes to pass.

So either the MediaWiki change has an issue or DonationInterface/ContributionTracking needs to be fixed/adjusted. Most probably the later.

The MediaWiki patch caused an issue in MassMessages and a revert was proposed with https://gerrit.wikimedia.org/r/#/c/263207/ (now abandoned).

The fix for MassMessage 80d4862660e72d933de5d1aa58c6028d499e994a / https://gerrit.wikimedia.org/r/#/c/263209/

-                       new RequestContext
+                       RequestContext::getMain()

No such thing in DonationInterfaces though :/

Just a quick look: I think the problematic line in the test of DonationInterface itself is:
$this->setMwGlobals( 'wgRequest', new FauxRequest( $init, false ) );
https://github.com/wikimedia/mediawiki-extensions-DonationInterface/blob/f4fb166c53c1b4d201a498e1d4682dcd87f61e49/tests/Adapter/GatewayAdapterTest.php#L191

The better way of doing this, would be to stub the method, that uses the global (I suspect ContextSource:.getRequest()), instead of stubbing the global itself. This would be more robust against changes inside of a specific function, that really needs to be stubbed (like we see now, RequestContext::getRequest() doesn't rely on the global anymore).

I haven't found the usage of RequestContext::getRequest() after some minutes of research and doesn't have the time (now) to investigate, maybe someone else want to take a look, otherwise I'll to it later :)

Btw.: This shouldn't impact production behavior, because the global is set to the default value of the "main" RequestContext instance while MediaWiki is initializing. This looks simply like a problem inside the test case (at far as DonationInterface doesn't do something scary with the wgRequest global in the functions code itself :/).

Florian claimed this task.Jan 11 2016, 2:47 PM

Change 263351 had a related patch set uploaded (by Florianschmidtwelzow):
Follow up mediawiki/core change

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

Just as a reference: The usage of Webrequest object, which is used to test functionality in the test case, is in the constructor of the GatewayAdapter:
https://github.com/wikimedia/mediawiki-extensions-DonationInterface/blob/bd82afcff56ca27be66c9c6185edfe5130c0eced/gateway_common/gateway.adapter.php#L400

used in this code block of session_resetOnSwitch():
https://github.com/wikimedia/mediawiki-extensions-DonationInterface/blob/bd82afcff56ca27be66c9c6185edfe5130c0eced/gateway_common/gateway.adapter.php#L3230-L3250

I don't know, if this is the correct usage, and don't know if my uploaded change fixes it, will work on this later.

Change 263362 had a related patch set uploaded (by Paladox):
Wipe out $wgRequest

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

Change 263351 abandoned by Florianschmidtwelzow:
Follow up mediawiki/core change

Reason:
see I671ca0641e622f6c4bac1dfa3fb17f9f3dd86f18

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

Change 263362 merged by jenkins-bot:
Wipe out $wgRequest

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

Ejegg renamed this task from DonationInterface fails mwext-DonationInterface-testextension-zend test to DonationInterface CI tests fail due to core change.
Ejegg closed this task as Resolved.
Ejegg edited a custom field.
mmodell removed a subscriber: awight.Jun 22 2017, 9:42 PM