Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC] {Test} Minor cleanup #21116

Merged
merged 1 commit into from
Aug 13, 2021
Merged

[NFC] {Test} Minor cleanup #21116

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions tests/phpunit/api/v3/ContributionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ class api_v3_ContributionTest extends CiviUnitTestCase {

/**
* Setup function.
*
* @throws \CiviCRM_API3_Exception
*/
public function setUp(): void {
parent::setUp();
Expand Down Expand Up @@ -120,8 +118,8 @@ public function setUp(): void {
/**
* Clean up after each test.
*
* @throws \API_Exception
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public function tearDown(): void {
$this->quickCleanUpFinancialEntities();
Expand All @@ -144,8 +142,6 @@ public function tearDown(): void {

/**
* Test Get.
*
* @throws \CRM_Core_Exception
*/
public function testGetContribution(): void {
$this->enableTaxAndInvoicing();
Expand Down Expand Up @@ -280,8 +276,6 @@ public function testCreateCheckContribution(): void {

/**
* Test the 'return' param works for all fields.
*
* @throws \CRM_Core_Exception
*/
public function testGetContributionReturnFunctionality(): void {
$params = $this->_params;
Expand Down Expand Up @@ -316,7 +310,7 @@ public function testGetContributionReturnFunctionality(): void {
// update contribution with invoice number
$params = array_merge($params, [
'id' => $contributionID,
'invoice_number' => CRM_Utils_Array::value('invoice_prefix', Civi::settings()->get('contribution_invoice_settings')) . "" . $contributionID,
'invoice_number' => Civi::settings()->get('invoice_prefix') . $contributionID,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the original was copy/paste from elsewhere, hence the extra . "". In other languages I've seen sometimes you need to do this type of thing to force string, since maybe a prefix like 0001 got output as just 1, although that doesn't seem to be true in php. But maybe something along those lines, I just can't think what.

But specifically for this test, isn't the prefix always null and known to be such? Not a blocker just was probably also part of copy/paste in the original.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@demeritcowboy yeah I guess if I'd thought it through further I would have removed it

'trxn_id' => 12345,
'invoice_id' => 6789,
]);
Expand Down Expand Up @@ -1288,7 +1282,7 @@ public function testCreateContributionPendingOnline() {
$this->assertEquals($contribution['values'][$contribution['id']]['trxn_id'], 12345);
$this->assertEquals($contribution['values'][$contribution['id']]['invoice_id'], 67890);
$this->assertEquals($contribution['values'][$contribution['id']]['source'], 'SSF');
$this->assertEquals($contribution['values'][$contribution['id']]['contribution_status_id'], 2);
$this->assertEquals(2, $contribution['values'][$contribution['id']]['contribution_status_id']);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - I realise all the lines above need the same change - I decided after the first one not to get carried away

$this->_checkFinancialRecords($contribution, 'pending');
}

Expand Down