From b17107c31a4cd4a21367122ca8610966db833ab5 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 16 Sep 2021 20:13:41 +1200
Subject: [PATCH] [NFC] [test] minor cleanup in test

This switches to using restoreMembershipTypes rather than creating a membership.

restoreMembershipTypes would ideally be in tearDown to get us back to a known membership
set but there is fallout
---
 .../CRM/Core/Payment/AuthorizeNetIPNTest.php  | 27 ++++++++++++++-----
 .../CRMTraits/Financial/OrderTrait.php        |  3 +--
 tests/phpunit/CiviTest/CiviUnitTestCase.php   |  2 +-
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
index 1b25b6c19c..5dbd417d97 100644
--- a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
+++ b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
@@ -10,6 +10,16 @@ use Civi\Api4\Contribution;
 class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
   use CRMTraits_Financial_OrderTrait;
 
+  /**
+   * Should financials be checked after the test but before tear down.
+   *
+   * Ideally all tests (or at least all that call any financial api calls ) should do this but there
+   * are some test data issues and some real bugs currently blocking.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = TRUE;
+
   protected $_contributionID;
   protected $_invoiceID = 'c2r9c15f7be20b4f3fef1f77e4c37424';
   protected $_financialTypeID = 1;
@@ -41,16 +51,19 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
 
   public function tearDown(): void {
     $this->quickCleanUpFinancialEntities();
+    $this->restoreMembershipTypes();
+    parent::tearDown();
   }
 
   /**
    * Ensure recurring contributions from Contribution Pages
    * with receipt turned off don't send a receipt.
    *
-   * @throws \CiviCRM_API3_Exception
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public function testIPNPaymentRecurNoReceipt() {
+  public function testIPNPaymentRecurNoReceipt(): void {
     $mut = new CiviMailUtils($this, TRUE);
     // Turn off receipts in contribution page.
     $api_params = [
@@ -107,7 +120,7 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
     catch (PaymentProcessorException $e) {
       $this->markTestSkipped('Error from A.net - cannot proceed');
     }
-    $this->_contributionID = $contribution->id;
+
     $this->ids['Contribution'][0] = $contribution->id;
     $this->_contributionRecurID = $contribution->contribution_recur_id;
 
@@ -148,7 +161,7 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testIPNPaymentRecurSuccess() {
+  public function testIPNPaymentRecurSuccess(): void {
     CRM_Core_BAO_ConfigSetting::enableComponent('CiviCampaign');
     $this->setupRecurringPaymentProcessorTransaction();
     $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurTransaction());
@@ -189,7 +202,7 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
     $this->assertEquals(1, $contribution['contribution_status_id']);
     $this->assertEquals('6511143069', $contribution['trxn_id']);
     // source gets set by processor
-    $this->assertTrue(substr($contribution['contribution_source'], 0, 20) == "Online Contribution:");
+    $this->assertEquals('Online Contribution:', substr($contribution['contribution_source'], 0, 20));
     $contributionRecur = $this->callAPISuccess('contribution_recur', 'getsingle', ['id' => $this->_contributionRecurID]);
     $this->assertEquals(5, $contributionRecur['contribution_status_id']);
   }
@@ -197,7 +210,9 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
   /**
    * Test IPN response updates contribution_recur & contribution for first & second contribution
    *
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
   public function testIPNPaymentRecurSuccessSuppliedReceiveDate() {
     $this->setupRecurringPaymentProcessorTransaction();
@@ -207,7 +222,7 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
     $this->assertEquals(1, $contribution['contribution_status_id']);
     $this->assertEquals('6511143069', $contribution['trxn_id']);
     // source gets set by processor
-    $this->assertTrue(substr($contribution['contribution_source'], 0, 20) == "Online Contribution:");
+    $this->assertEquals('Online Contribution:', substr($contribution['contribution_source'], 0, 20));
     $contributionRecur = $this->callAPISuccess('contribution_recur', 'getsingle', ['id' => $this->_contributionRecurID]);
     $this->assertEquals(5, $contributionRecur['contribution_status_id']);
     $IPN = new CRM_Core_Payment_AuthorizeNetIPN(array_merge(['receive_date' => '2010-07-01'], $this->getRecurSubsequentTransaction()));
diff --git a/tests/phpunit/CRMTraits/Financial/OrderTrait.php b/tests/phpunit/CRMTraits/Financial/OrderTrait.php
index f3c61ac053..18923f4b28 100644
--- a/tests/phpunit/CRMTraits/Financial/OrderTrait.php
+++ b/tests/phpunit/CRMTraits/Financial/OrderTrait.php
@@ -28,7 +28,6 @@ trait CRMTraits_Financial_OrderTrait {
   public function createRepeatMembershipOrder(): void {
     $this->createExtraneousContribution();
     $this->ids['contact'][0] = $this->individualCreate();
-    $this->ids['membership_type'][0] = $this->membershipTypeCreate();
 
     $contributionRecur = $this->callAPISuccess('ContributionRecur', 'create', array_merge([
       'contact_id' => $this->_contactID,
@@ -59,7 +58,7 @@ trait CRMTraits_Financial_OrderTrait {
         [
           'params' => [
             'contact_id' => $this->ids['contact'][0],
-            'membership_type_id' => $this->ids['membership_type'][0],
+            'membership_type_id:name' => 'General',
             'contribution_recur_id' => $contributionRecur['id'],
             'source' => 'Payment',
           ],
diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index b1611dff83..06580a382b 100644
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -2582,7 +2582,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase {
    * @param array $recurParams (Optional)
    * @param array $contributionParams (Optional)
    */
-  public function setupRecurringPaymentProcessorTransaction($recurParams = [], $contributionParams = []): void {
+  public function setupRecurringPaymentProcessorTransaction(array $recurParams = [], array $contributionParams = []): void {
     $this->ids['campaign'][0] = $this->callAPISuccess('Campaign', 'create', ['title' => 'get the money'])['id'];
     $contributionParams = array_merge([
       'total_amount' => '200',
-- 
2.25.1