alexa-js / alexa-verifier-middleware

An express middleware that verifies HTTP requests sent to an Alexa skill are sent from Amazon.
MIT License
31 stars 6 forks source link

Most tests in this module don't actually test a anything #16

Closed dblock closed 7 years ago

dblock commented 7 years ago

For example, the second one:

test('fail if request body is already parsed', function(t) {
  var mockReq = httpMocks.createRequest({
    method: 'POST',
    headers: { },
    _body: true,
    _rawBody: {},
    on: function(eventName, callback) { }
  })

  var mockRes = httpMocks.createResponse({
    status: function(httpCode) {
      t.equal(httpCode, 400)
      return {
        json: function(input) {
          t.deepEqual(input, { status: 'failure', reason: 'The raw request body is not available.' })
        }
      }
    }
  })

  var nextInvocationCount = 0
  var mockNext = function() { nextInvocationCount++ }

  verifier(mockReq, mockRes, mockNext)

  t.equal(nextInvocationCount, 0)
  t.end()
})

It never invokes that status function or compare the equal json. The fix here is to rename _rawBody to rawBody, that is probably a typo, and to just compare the output of mockRes:

test('fail if request body is already parsed', function(t) {
  var mockReq = httpMocks.createRequest({
    method: 'POST',
    headers: {},
    _body: true,
    rawBody: {}
  })

  var responseHttpCode;

  var mockRes = httpMocks.createResponse();

  var nextInvocationCount = 0
  var mockNext = function() { nextInvocationCount++ }

  verifier(mockReq, mockRes, mockNext)

  t.equal(mockRes.statusCode, 400)
  t.deepEqual(JSON.parse(mockRes._getData()), {
    reason: "The raw request body is not available.",
    status: "failure"
  });

  t.equal(nextInvocationCount, 0)
  t.end()
})

I started fixing others, however maybe there's abuse of mocks here IMO and actually mounting the middleware on express and invoking it would make tests much shorter and clearer?