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

CRM-21682 Membership Renewal Tests #11824

Merged

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Mar 16, 2018

Overview

Existing membership renewal tests are testing renewals where duration units don't match (ie. recur has a monthly interval but membership has a yearly interval). This is an unlikely real-world scenario as it would lead to an annual membership being renewed by a year every month that the recurring contribution is renewed!

This PR updates the existing unit tests so they use matching frequencies for membership and recurring contributions. It then extends those tests so the same tests can be run with different frequencies, improving overall test coverage.

Before

Tests are not testing a valid real-world scenario (It might be nice to pay in monthly installments for an annual membership but that's not what the code is testing and is not how it currently works).

After

Test coverage is improved. Real-world renewal scenarios are tested.

Comments

Improve test coverage to support changes in #11556.


@mattwire mattwire force-pushed the CRM-21682_membership_renewal_tests branch from 2dd7dd1 to 8574a06 Compare March 16, 2018 17:29
@mattwire mattwire force-pushed the CRM-21682_membership_renewal_tests branch from 8574a06 to 5d6abb7 Compare March 21, 2018 11:20
@mattwire mattwire force-pushed the CRM-21682_membership_renewal_tests branch from 5d6abb7 to b82d087 Compare March 21, 2018 12:39
@eileenmcnaughton
Copy link
Contributor

Seems reasonable & well thought out. Only affects tests - merging

@eileenmcnaughton eileenmcnaughton merged commit d8548fb into civicrm:master Mar 23, 2018
@mattwire mattwire deleted the CRM-21682_membership_renewal_tests branch September 25, 2018 11:04
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.

3 participants