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.

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

Test fixes, towards CRM-17647 #11574

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jan 23, 2018

Overview

Test fixes towards never calling Contribution::Create without skipCleanMoney

Before

tests pass

After

tests pass. Less places where Contribution::create called without skipCleanMoney (via the api)

Technical Details

Towards #11539

Comments


Test fixes towards never calling Contribution::Create without skipCleanMoney

$this->assertEquals($params['trxn_id'], $contribution->trxn_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton are we loosing anything by removing these asserts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so -creating contributions is tested elsewhere & these are tangental to what is being tested

@seamuslee001
Copy link
Contributor

I'm satisfied with the changes and reviewed to ensure that the tests aren't being reduced in a meaningful way. Happy for this to be merged now noting it only affects tests and the test suite has passed.

@eileenmcnaughton eileenmcnaughton merged commit 5e5f37d into civicrm:master Jan 23, 2018
@eileenmcnaughton
Copy link
Contributor Author

thanks @seamuslee001

@eileenmcnaughton eileenmcnaughton deleted the test_fixes branch January 23, 2018 20:31
@mlutfy mlutfy added this to the 4.7.31 milestone Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants