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

(WIP) Fix PayPal test as it appears contributions are now getting set to be… #9041

Closed
wants to merge 2 commits into from

Conversation

seamuslee001
Copy link
Contributor

… completed and not pending

@totten @eileenmcnaughton @monishdeb @jitendrapurohit I'm not sure why this has happened but this fixes the test errors we have been seeing

@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@eileenmcnaughton
Copy link
Contributor

hmm that gives me the heeby jeebies - can we see what commit started it failing?

@monishdeb
Copy link
Member

Unable to find the commit but as per the PR's test build failure it started to fail 6 days ago - 9th Sept. And this unit test testSubmitCreditCardPayPal was introduced in 22nd Aug. Don't what causes it to fail

@monishdeb
Copy link
Member

monishdeb commented Sep 15, 2016

Morever I gone through the entire workflow where it interacts with paypal for CC payment ( here https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/PayPalImpl.php#L995 )
and I found clean $response for desired unit test

{"receiverbusiness":"sunil._1183377782_biz@webaccess.co.in","receiveremail":"sunil._1183377782_biz@webaccess.co.in","receiverid":"BNCETES6EECQQ","payerid":"DNU49C86RUU2A","payerstatus":"unverified","countrycode":"US","addressowner":"PayPal","addressstatus":"None","invnum":"ef3bbe9423598e7068fe054dfa3b83e2","salestax":"0.00","timestamp":"2016-09-15T13:38:56Z","correlationid":"6bb0ea10a1bfd","ack":"Success","version":"3","build":"000000","firstname":"Junko","lastname":"Adams","transactionid":"71Y80036DE2246723","receiptid":"2330-9021-1299-9705","transactiontype":"webaccept","paymenttype":"instant","ordertime":"2016-09-15T13:38:53Z","amt":"50.00","feeamt":"1.75","taxamt":"0.00","currencycode":"USD","paymentstatus":" Completed ","pendingreason":"None","reasoncode":"None","l_name0":"Contribution submitted by a staff person using contributor's credit card","l_qty0":"1","l_taxamt0":"0.00","l_currencycode0":"USD","l_amt0":"50.00"} . So I think this PR can be merged @eileenmcnaughton ?

@totten
Copy link
Member

totten commented Sep 15, 2016

I have no idea what the correct behavior is here... but a fun fact...

I checked out copies of 4.7.10 and 4.7.9 and ran the tests - it reports the same problem as master (CRM_Contribute_Form_ContributionTest::testSubmitCreditCardPayPal -- incorrect count returned from Contribution getcount).

@colemanw
Copy link
Member

This one failing test is really harshing my mellow when reviewing PRs. Could we just mark it as incomplete for now?

@seamuslee001
Copy link
Contributor Author

@colemanw I have just fired in a PR to do that #9144 I will then update this PR to be based off that i.e. removing marking as incomplete but i think we should keep this PR alive whilst Eileen gets a chance to investigate if this is a legit change to the test suite or something else is up

@seamuslee001 seamuslee001 changed the title Fix PayPal test as it appears contributions are now getting set to be… (WIP) Fix PayPal test as it appears contributions are now getting set to be… Sep 29, 2016
@colemanw colemanw closed this Oct 12, 2016
@colemanw
Copy link
Member

Tests are now passing. Do we still need this PR?

@seamuslee001
Copy link
Contributor Author

@colemanw i believe we do as we are currently just skipping this test, We need to work out if this is indeed a change to the test or not and get the test re-running

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants