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

Added further tests for process_membership job: check status for inherited memberships #12927

Merged

Conversation

davejenx
Copy link
Contributor

Overview

Added further checks for process_membership job: check that status is updated correctly for inherited membership. In function testProcessMembershipUpdateStatus.

Before

No tests for inherited membership processing.

After

Tests for inherited membership processing.

Technical Details

In existing function testProcessMembershipUpdateStatus.

Comments

Test was failing during development with inherited membership being deleted. Tracked this down to a static variable in CRM_Member_BAO_Membership::createRelatedMemberships(). There is a reset parameter for this static, so I used that to work around the problem. The issue should not occur outside tests. An alternative approach would be to replace the static variable with Civi::$statics but note reset facility is used elsewhere: CRM-17723. See CRM-4213, CRM-19735.

@civibot
Copy link

civibot bot commented Oct 11, 2018

(Standard links)

@civibot civibot bot added the master label Oct 11, 2018
@davejenx
Copy link
Contributor Author

@eileenmcnaughton The next instalment!

@davejenx davejenx changed the title Added further checks for process_membership job: check status for inherited memberships Added further tests for process_membership job: check status for inherited memberships Oct 11, 2018
@eileenmcnaughton
Copy link
Contributor

@davejenx there is a style issue

@davejenx
Copy link
Contributor Author

@eileenmcnaughton Oops, accidentally committed cherry-picked stuff too, please don't merge until I've fixed!

@davejenx
Copy link
Contributor Author

OK, that's reverted now. Think we're OK.

@eileenmcnaughton
Copy link
Contributor

@davejenx - do you want to rebase -i to drop them out & squash the patches?

@davejenx
Copy link
Contributor Author

@eileenmcnaughton I'm not familiar with how that's done.

… updated correctly for inherited membership. In function testProcessMembershipUpdateStatus.
@davejenx davejenx force-pushed the job_process_memberships_test_inherit branch from 90ffef0 to 0c6bcd7 Compare October 11, 2018 20:21
@davejenx
Copy link
Contributor Author

@eileenmcnaughton Howzat?

@eileenmcnaughton
Copy link
Contributor

spot on

@eileenmcnaughton eileenmcnaughton merged commit 55af0ec into civicrm:master Oct 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants