Tax fixes in unit test #20390
Merged
Tax fixes in unit test #20390
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
| @@ -549,7 +558,7 @@ public function testCreateContributionChainedLineItems() { | |||
| 'trxn_id' => 12345, | |||
| 'invoice_id' => 67890, | |||
| 'source' => 'SSF', | |||
| 'contribution_status_id' => 1, | |||
| 'contribution_status_id' => 'Pending', | |||
There was a problem hiding this comment.
Switch to pending to create a valid payment flow - does not affect what is being tested (which should never be done in code - use the order api)
eileenmcnaughton
force-pushed
the
tax2
branch
2 times, most recently
from
f3065d1 to
41723dd
Compare
| @@ -2060,6 +2062,7 @@ public static function submit($params) { | |||
| $paramsProcessedForForm = $form->_params = self::getFormParams($params['id'], $params); | |||
There was a problem hiding this comment.
This function (submit()) is used primary by unit tests via the ContributionPage.submit api. AFAIK that api is only used by a barage of tests but there might be some live usages in the ether - I would expect the test cover to be greater than any possible 'wild usage'
|
Reviewed code, looks good. Tested on local. |
|
Test fails relate @eileenmcnaughton |
eileenmcnaughton
force-pushed
the
tax2
branch
4 times, most recently
from
265eaf8 to
8738175
Compare
When this->isValidateFinancialsOnPostAssert is true the test class checks that line items and payments are valid. I'm trying to enable this for this class. However, there are some issues that I have found fixes for (and at least 1 I'm still working on) - some tests try to set tax_amount when it is not enabled which is invalid - removed - one test tries to use chaining in a way that we know is not going to do a job of creating the entities as it adds the payment before the line items. I switched this to create a pending payment which doesn't alter the thing under test & brings it closer to the recommended flow - one test is deliberately invalid - I marked it as not eligible for the validation - the price set id was not being passed to the Confirm->submit function (accessed by tests, mostly via the ContributionPage.submit api) - I added functionality to retrieve it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Tax fixes in unit test
When this->isValidateFinancialsOnPostAssert is true the
test class checks that line items and payments are valid before tear down.
I'm trying to enable this for this class. However, there are some issues
that I have found fixes for (and at least 1 I'm still working on)
which is invalid - removed
we know is not going to do a job of creating the entities
as it adds the payment before the line items. I switched
this to create a pending payment which doesn't alter the
thing under test & brings it closer to the
recommended flow
not eligible for the validation
function (accessed by tests, mostly via the ContributionPage.submit
api) - I added functionality to retrieve it
Before
Taxes affected by the above fail when we increase financial validation
After
Those tests pass
Technical Details
Some or possibly all of the failures in #20357 relate to issues with the tests
Comments