recurly / recurly-client-node

node.js library for Recurly's V3 API
https://developers.recurly.com
MIT License
44 stars 10 forks source link

Guidance on testing #46

Closed rafiyagi closed 5 years ago

rafiyagi commented 5 years ago

What are best practices for mocking and stubbing the recurly client?

I'm using a combination of chai.js, mocha.js and sinon.js for testing. I'm using sinon.js to stub the recurly client so I can return my own stubbed objects- however when I look at what the actual client returns I see there are a lot of instantiated resource objects that are dynamically created.

For example the response from getBillingInfo is a a variety of resource objects like BillingInfo, Address, PaymentMethod and BillingInfoUpdatedBy objects

My question is: should I try to mimic these objects on my own via fixtures?

Or should I use a package like nock to hijack HTTP request and try and emulate recurly's v3 json responses?

Thanks!

rafiyagi commented 5 years ago

Or maybe there is another way of doing it that I have not thought about!

bhelx commented 5 years ago

@rafdizzle86 Great question! I expect to eventually provide testing tools and advice in all of the libraries. For now, I'll try to respond with my philosophy and some example code here.

For starters, I'd not recommend mocking on an HTTP level. Unlike the V2 clients, we provide you with a small interface where all the logic lives (the Client class). The client methods are the interface (or boundary) between us, and the resources are the messages. You want to test the interface, not the implementation. Not only is mocking the HTTP calls reaching through this interface, I find it to be more painful to setup and keep up to date. Instead, mock the client methods!

As an example of testing getBillingInfo Let's suppose we have this object and method which, given an account code, can return a billing info id. It returns null for 404s. Quite contrived I know, but we need some fake application code to test.

class MyRecurlyAction {
  constructor(recurlyClient) {
    this.recurlyClient = recurlyClient
  }

  async getBillingInfoId(accountCode) {
    const accountId = `code-${accountCode}`
    try {
      const billingInfo = await this.recurlyClient.getBillingInfo(accountId)
      return billingInfo.id
    } catch (err) {
      if (err && err instanceof recurly.errors.NotFoundError) {
        return null
      }
      throw err
    }
  }
}

To test this, I'd recommend creating a stubbed client of some sort. keep in mind I'm still very unsure how to use sinon the way i want:

function createStubbedClient() {
  let client = new recurly.Client('testapikey')
  // Stub getBillingInfo
  client.getBillingInfo = sinon.stub()
  // You could create a BillingInfo object in js,
  // or use a json object as a fixture
  let billingInfoObj = JSON.parse(fs.readFileSync('./test/fixtures/billing_info/code-verena.json'))
  // recursively cast the POJO into a billing info
  let billingInfo = recurly.BillingInfo.cast(billingInfoObj)
  // use resolves and not `returns` here because the client methods return promises
  client.getBillingInfo.resolves(billingInfo)
  // Stub other methods you might need here...
  return client
}

Now your mocha tests:

describe('MyRecurlyAction', () => {
  describe('getBillingInfoId', () => {
    it('with a good billing info', (done) =>  {
      let stubbedClient = createStubbedClient()
      // inject the stubbed client
      let action = new MyRecurlyAction(stubbedClient)
      action.getBillingInfoId("verena")
        .then(id => {
          // assert that we called the client with the right arguments (code-verena)
          // notice how we are asserting its called with "code-verena" and not "verena"
          // this tests that your code properly created the accountid from the code (line: `code-${accountCode}`)
          // your logic might be much more complicated than this in reality 
          sinon.assert.calledWith(stubbedClient.getBillingInfo, "code-verena");
          // assert the return value
          // this tests that your code properly turned the recurly response
          // into what you want
          assert.equal(id, "lns8m3mkqj7u")
          done()
        })
        .catch(done)
    })
  })
})

Some important notes to have complete tests here. You MUST do both of these:

  1. Assert that the stub is called with the correct arguments for all cases. This tests that your code is crafting the correct request to recurly.
  2. Assert that your code correctly handles the responses for all cases.

So, what about the other cases for this test (like the 404 case)? I'm still not sure how to do this with sinon, but ideally you could create a stubbed client that changes behavior depending on the arguments. Here's a madeup chaining syntax:

let billingInfoObj = JSON.parse(fs.readFileSync('./test/fixtures/billing_info/code-verena.json'))
let billingInfo = recurly.BillingInfo.cast(billingInfoObj)

client.getBillingInfo = sinon.stub()

client.getBillingInfo
  .calledWith("code-good-account").resolves(billingInfo)
  .calledWith("code-bad-account").throws(recurly.errors.NotFoundError)

You would then do this for all the methods and scenarios you wish to test. This stubbed client could also be plugged into your application for over-the-shoulder testing or UI testing. Again, I'm unsure how to do this in sinon but it's a thing in other languages frameworks. If it's not possible in sinon, you could also use some kind of strategy pattern to inject the behavior when you create the stub.

bhelx commented 5 years ago

BTW, in regards to your question here:

For example the response from getBillingInfo is a a variety of resource objects like BillingInfo, Address, PaymentMethod and BillingInfoUpdatedBy objects

If you use the cast function on any resource class, it will recursively cast the whole object and it's attributes. Example:

let billingInfoObj = JSON.parse(fs.readFileSync('./test/fixtures/billing_info/code-verena.json'))
let billingInfo = recurly.BillingInfo.cast(billingInfoObj)

The classes are aware of their schema and able to do this casting accurately.

rafiyagi commented 5 years ago

That helps, thanks again @bhelx!