safaricom / mpesa-node-library

M-Pesa Library for Node.js using REST API
Apache License 2.0
161 stars 139 forks source link

Prevent C2B in production #6

Closed MarkNjunge closed 6 years ago

MarkNjunge commented 6 years ago

Fixes #5

geofmureithi-zz commented 6 years ago

Hey, Looks good, but seems travis is not configured well which I will setup ASAP. Meanwhile, why dont you add a test in *mpesa-node-library/tests/unit/. See https://github.com/safaricom/mpesa-node-library/blob/master/tests/unit/index.test.js as an example

MarkNjunge commented 6 years ago

Will do.

MarkNjunge commented 6 years ago

I've added a commit. I'm not sure if it notified you.

MarkNjunge commented 6 years ago

Following the documentation, bind will cause this in m-pesa.js to refer to the productionInstance. Without it, this will refer to an object created by the test, which does not have the config.

Therefore,

expect(productionInstance.c2bSimulate).throwError(error => {
  expect(error.message).to.be('Cannot call C2B simulate in production.')
})

Will result in the test failing because

  1) C2B in production
       should throw error:
     Error: expected 'Cannot read property \'environment\' of undefined' to equal 'Cannot call C2B simulate in production.'
      at Assertion.assert (node_modules\expect.js\index.js:96:13)
      at Assertion.be.Assertion.equal (node_modules\expect.js\index.js:216:10)
      at Assertion.(anonymous function) [as be] (node_modules\expect.js\index.js:69:24)
      at expect.throwError.error (tests\unit\index.test.js:44:32)
      at Assertion.throwError.Assertion.throwException (node_modules\expect.js\index.js:159:9)
      at Context.<anonymous> (tests\unit\index.test.js:43:44)

The other way to do it is to wrap the method call in a try catch.

geofmureithi-zz commented 6 years ago

I see. After some research, I have seen how the throwError behaves, this becomes better:

it('should throw error', function () {
    let threwError = false;
    const productionInstance = new Mpesa({
      consumerKey: 'test',
      consumerSecret: 'test',
      environment: 'production'
    })
    try {
      productionInstance.c2bSimulate()
    } catch (e) {
      threwError = true;
      expect(e.message).to.be('Cannot call C2B simulate in production.')
    } finally {
      expect(threwError).to.be(true)
    }

Also, use: if(this.enviroment === 'production') throw new Error('Cannot call C2B simulate in production.') Additionally it would be good to test the non-production scenario too. Good work though!.

geofmureithi-zz commented 6 years ago

To be merged tonight.