jenkins-bot has submitted this change and it was merged.

Change subject: Test failure path
......................................................................


Test failure path

Pulls a few things into helper functions that can get overridden
This should be the same idea as
I5bafd006cee05c33ce3089a7d844e03ce3ae3a8d

Change-Id: I32c2c547d62c4333763542eb4a62417af3a97a0c
---
M PaymentProviders/PayPal/Job.php
M PaymentProviders/PayPal/PayPalPaymentsAPI.php
M PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php
M PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php
4 files changed, 39 insertions(+), 15 deletions(-)

Approvals:
  Awight: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/PaymentProviders/PayPal/Job.php b/PaymentProviders/PayPal/Job.php
index 90dd9a2..c1b7711 100644
--- a/PaymentProviders/PayPal/Job.php
+++ b/PaymentProviders/PayPal/Job.php
@@ -5,6 +5,8 @@
 
 class Job extends RunnableJob {
 
+       static $verifyFailedMsg = 'PayPal message verification failed';
+
        public $payload;
 
        public function execute() {
@@ -16,11 +18,10 @@
 
                // XXX Why does everything get made into objects?
                $request = (array)$this->payload;
-               $request['cmd'] = '_notify-validate';
 
                $valid = $this->config->object( 'api' )->validate( $request );
                if ( ! $valid ) {
-                       throw new \Exception( 'PayPal message verification 
failed' );
+                       throw new \Exception( self::$verifyFailedMsg );
                }
 
                // Determine message type.
diff --git a/PaymentProviders/PayPal/PayPalPaymentsAPI.php 
b/PaymentProviders/PayPal/PayPalPaymentsAPI.php
index 9a8520a..272e857 100644
--- a/PaymentProviders/PayPal/PayPalPaymentsAPI.php
+++ b/PaymentProviders/PayPal/PayPalPaymentsAPI.php
@@ -5,7 +5,9 @@
 class PayPalPaymentsAPI {
 
        // Simply a function to override in testing.
-       protected function curl ( $ch ) {
+       protected function curl ( $ch, $post_fields ) {
+               $post_fields['cmd'] = '_notify-validate';
+               curl_setopt( $ch, CURLOPT_POSTFIELDS, $post_fields );
                return curl_exec( $ch );
        }
 
@@ -21,8 +23,7 @@
                curl_setopt( $ch, CURLOPT_FOLLOWLOCATION, 0 );
                curl_setopt( $ch, CURLOPT_RETURNTRANSFER, 1 );
                curl_setopt( $ch, CURLOPT_POST, 1 );
-               curl_setopt( $ch, CURLOPT_POSTFIELDS, $post_fields );
                // TODO we can put VERIFIED in config and generalize this
-               return $this->curl( $ch ) === 'VERIFIED';
+               return $this->curl( $ch, $post_fields ) === 'VERIFIED';
        }
 }
diff --git a/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php 
b/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php
index da3fd0c..74ed15b 100644
--- a/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php
+++ b/PaymentProviders/PayPal/Tests/MockPayPalPaymentsAPI.php
@@ -3,7 +3,10 @@
 use SmashPig\PaymentProviders\PayPal\PayPalPaymentsAPI;
 
 class MockPayPalPaymentsAPI extends PayPalPaymentsAPI {
-       protected function curl () {
-               return 'VERIFIED';
+       protected function curl ( $ch, $post_fields ) {
+               // XXX Not sure if too twisted.
+               if ( CaptureIncomingMessageTest::$verified_msg === $post_fields 
) {
+                       return 'VERIFIED';
+               }
        }
 }
diff --git 
a/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php 
b/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php
index 36fa095..740acdf 100644
--- a/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php
+++ b/PaymentProviders/PayPal/Tests/phpunit/CaptureIncomingMessageTest.php
@@ -21,13 +21,13 @@
         * @var Configuration
         */
        public $config;
-       private $verified_msg;
+       static $verified_msg;
 
        public function setUp() {
                parent::setUp();
                $this->config = PayPalTestConfiguration::get();
                Context::initWithLogger( $this->config );
-               $this->verified_msg = json_decode(
+               self::$verified_msg = json_decode(
                        file_get_contents( __DIR__ . '/../Data/web_accept.json' 
),
                        true
                );
@@ -42,7 +42,7 @@
 
        public function testCapture() {
 
-               $this->capture( $this->verified_msg );
+               $this->capture( self::$verified_msg );
 
                // TODO why get it from BaseQueueConsumer instead of config?
                $jobQueue = BaseQueueConsumer::getQueue( 'jobs-paypal' );
@@ -51,13 +51,13 @@
                $this->assertEquals( $jobMessage['php-message-class'],
                        'SmashPig\PaymentProviders\PayPal\Job' );
 
-               $this->assertEquals( $jobMessage['payload'], 
$this->verified_msg );
+               $this->assertEquals( $jobMessage['payload'], 
self::$verified_msg );
 
        }
 
        public function testConsume () {
 
-               $this->capture( $this->verified_msg );
+               $this->capture( self::$verified_msg );
 
                // TODO DRY?
                $jobQueue = BaseQueueConsumer::getQueue( 'jobs-paypal' );
@@ -73,9 +73,28 @@
                $verifiedQueue = $this->config->object( 'data-store/verified' );
                $verifiedMessage = $verifiedQueue->pop();
 
-               // TODO can we verify that it looks right after 
transmogrification? might be
-               // a job for the crm consumer
-               $this->assertTrue( ! empty( $verifiedMessage ) );
+               $this->assertNotEmpty( $verifiedMessage );
+
+       }
+
+       public function testFailedConsume () {
+               $jobMessage = array('just' => 'some', 'old' => 'message' );
+               $jobClass = 'SmashPig\PaymentProviders\PayPal\Job';
+               $job = KeyedOpaqueStorableObject::fromJsonProxy(
+                       $jobClass,
+                       json_encode( $jobMessage )
+               );
+
+               try {
+                       $job->execute();
+               } catch ( \Exception $e ) {
+                       // TODO I think this can throw a special exception to 
move to
+                       // damaged queue or some other stuff
+                       $this->assertEquals(
+                               
\SmashPig\PaymentProviders\PayPal\Job::$verifyFailedMsg,
+                               $e->getMessage()
+                       );
+               }
 
        }
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/310942
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I32c2c547d62c4333763542eb4a62417af3a97a0c
Gerrit-PatchSet: 6
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to