Improve test coverage

Created on 12 August 2020, over 4 years ago
Updated 18 January 2023, almost 2 years ago

Problem/Motivation

I think it would be a good idea to work on adding more tests for this module. In particular I think it would help to improve confidence that we're not breaking things when committing patches and hopefully help improve the speed of bug fixes etc.

Additionally, I think it'll make it easier to do things like update the Recurly SDK to new versions if we know our tests will point out any issues. This would help with both the issue here [INSERT ISSUE] where we're trying to bump the PHP SDK version so that it's compatible with the latest V2 API. As well as a future issue in which we'll want to work on integrating with the V3 version of the Recurly API.

Proposed resolution

I'm thinking that for starters it would be good to provide functional (integration) tests that cover all the places where we rely on code from the Recurly PHP SDK. While this wouldn't necessarily always tell us exactly what broke, it would at least let us know that something did break. Then we can follow up with adding unit tests for more specific things like access control, the various string formatting utilities, and things like that.

Some of the work that needs to be done here is generic and required for all/most new tests. Like for example resolving #2141995: Recurly library's "$client" arguments not passed β†’ which currently prevents us from being able to create a mock API client. And, then providing some generic mocking code that individual tests can extend. And some of this is going to be just writing the specific test implementation.

Question. Does it make sense to work on this as one large patch. Or to open a bunch of individual issues for testing of various components? Maybe use this issue to implement the groundwork required to allow mocking API calls, and then implement tests to cover one feature of as an example. And then open new issues to cover other things? My concern is that if we have lots of issues it's a burden to coordinate. And if we have just one issue it's a burden to review what is likely to become a really large patch + refactor.

Remaining tasks

I've already started on implementing a proof of concept for creating a mock API client and the refactoring that would have to happen to allow that.

While not a complete list, after poking around a bit here are some of the things I think it would be good to ensure we cover:

  • Everything in src/Access
  • Entity Operations ... e.g. remote account is updated when entityUpdate is called
  • Redeem coupon form
  • Reactivate controller
  • \Drupal\recurly\RecurlyEntityTypeInfo::entityTypeAlter
  • User info mapping
  • recurly_user_cancel() & recurly_user_delete()
  • \recurly_account_load()
  • RouteSubscriber adds all the right routes to the configured entity type

As well as probably the signup and cancellation flows for both recurlyjs and recurly_hosted

✨ Feature request
Status

Needs work

Version

4.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States eojthebrave Minneapolis, MN

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Status changed to RTBC almost 2 years ago
  • Status changed to Needs work almost 2 years ago
Production build 0.71.5 2024