Project60 / org.project60.sepa

SEPA direct debit integration with civicrm
19 stars 46 forks source link

mandate reference collision test failing after fixing test (maybe, not sure) #649

Closed bbaermann closed 1 year ago

bbaermann commented 1 year ago

I am relatively new to PHP, so maybe all of the following is completely bollocks. But here is my understanding of the situation:

There was an error in two unit tests, testOOFFMandateReferenceCollision() and testRCURMandateReferenceCollision(). The variable $contactId was not accessible inside the scope of the closure. This leads to wrongly working of the test. If i correct the error in https://github.com/Project60/org.project60.sepa/commit/72804807655d3c764c264cbb88169a4f7839223b , than the tests faile.

So is there really a problem with reference collission or is this a problem with my ne test code?

bbaermann commented 1 year ago

maybe a bit misleading is that the old code is not in the diff above. it is here: https://github.com/Project60/org.project60.sepa/commit/341a356161e5e10a50343a2d84637ccac87a4fc1

bjendres commented 1 year ago

I have refactored those tests with https://github.com/Project60/org.project60.sepa/commit/c7fa93e858d411d9e5869d01383f2c6653eb17d6. I think they never worked in the first place, but they should now pass.

We'll close this when branch 645_php8 is merged.