Merge pull request #523 from apiaryio/honzajavorek/skip-non-2xx

Skipping non 2xx responses by default for Swagger

package.json

@@ -34,7 +34,7 @@
     "clone": "^1.0.0",
     "coffee-script": "^1.10.0",
     "colors": "^1.1.2",
-    "dredd-transactions": "^1.2.0",
+    "dredd-transactions": "^1.3.1",
     "file": "~0.2.2",
     "gavel": "0.5.2",
     "glob": "^6.0.1",

src/dredd.coffee

@@ -181,6 +181,7 @@ class Dredd
           warning.type = 'warning'
           fileData.annotations.push(warning)
 
+        fileData.mediaType = compilationResult.mediaType
         @transactions = @transactions.concat(compilationResult.transactions)
         next()
       )

src/transaction-runner.coffee

@@ -280,9 +280,9 @@ class TransactionRunner
 
   configureTransaction: (transaction, callback) =>
     configuration = @configuration
-    origin = transaction['origin']
-    request = transaction['request']
-    response = transaction['response']
+
+    {origin, request, response} = transaction
+    mediaType = configuration.data[origin.filename]?.mediaType or 'text/vnd.apiblueprint'
 
     # parse the server URL just once
     @parsedUrl ?= url.parse configuration['server']
@@ -312,11 +312,8 @@ class TransactionRunner
         headerKey = header.substring(0, splitIndex)
         headerValue = header.substring(splitIndex + 1)
         flatHeaders[headerKey] = headerValue
-
     request['headers'] = flatHeaders
 
-    id = request['method'] + ' ' + request['uri']
-
     # The data models as used here must conform to Gavel.js
     # as defined in `http-response.coffee`
     expected =
@@ -330,17 +327,25 @@ class TransactionRunner
     unless @multiBlueprint
       transaction.name = transaction.name.replace("#{transaction.origin.apiName} > ", "")
 
+    # Transaction skipping (can be modified in hooks). If the input format
+    # is Swagger, non-2xx transactions should be skipped by default.
+    skip = false
+    if mediaType.indexOf('swagger') isnt -1
+      status = parseInt(response.status, 10)
+      if status < 200 or status >= 300
+        skip = true
+
     configuredTransaction =
       name: transaction.name
-      id: id
+      id: request['method'] + ' ' + request['uri']
       host: @parsedUrl['hostname']
       port: @parsedUrl['port']
       request: request
       expected: expected
       origin: origin
       fullPath: fullPath
       protocol: @parsedUrl.protocol
-      skip: false
+      skip: skip
 
     return callback(null, configuredTransaction)
 

test/fixtures/multiple-responses.yaml

@@ -0,0 +1,40 @@
+swagger: "2.0"
+info:
+  version: "1.0"
+  title: Beehive API
+consumes:
+  - application/json
+produces:
+  - application/xml
+  - application/json
+paths:
+  /honey:
+    get:
+      responses:
+        400:
+          description: user error
+        500:
+          description: server error
+        200:
+          description: pet response
+          headers:
+            X-Test:
+              type: string
+              enum:
+                - Adam
+                - Pavan
+          schema:
+            $ref: '#/definitions/Bee'
+definitions:
+  Bee:
+    required:
+      - id
+      - name
+    properties:
+      id:
+        type: integer
+        format: int64
+      name:
+        type: string
+      tag:
+        type: string

test/fixtures/swagger-multiple-responses.js

@@ -0,0 +1,10 @@
+
+var hooks = require('hooks');
+
+
+hooks.before('/honey > GET', function (transaction, done) {
+  if (transaction.expected.statusCode[0] == '5') {
+    transaction.skip = false;
+  }
+  done();
+});

test/integration/dredd-test.coffee

@@ -46,7 +46,7 @@ execCommand = (options = {}, cb) ->
   return
 
 
-describe "Dredd class Integration", () ->
+describe 'Dredd class Integration', ->
   dreddCommand = null
   custom = {}
 
@@ -550,3 +550,62 @@ describe "Dredd class Integration", () ->
 
       it 'should execute hook with fuxed name', () ->
         assert.include stderr, 'Fixed transaction name'
+
+  describe('when Swagger document has multiple responses', ->
+    reTransaction = /(\w+): (\w+) \/honey/g
+    matches = undefined
+
+    beforeEach((done) ->
+      execCommand(
+        options:
+          path: './test/fixtures/multiple-responses.yaml'
+      , (err) ->
+        matches = []
+        matches.push(groups) while groups = reTransaction.exec(stdout)
+        done(err)
+      )
+    )
+
+    it('recognizes all 3 transactions', ->
+      assert.equal(matches.length, 3)
+    )
+    it('the transaction #1 is skipped', ->
+      assert.equal(matches[0][1], 'skip')
+    )
+    it('the transaction #2 is skipped', ->
+      assert.equal(matches[1][1], 'skip')
+    )
+    it('the transaction #3 is not skipped (status 200)', ->
+      assert.notEqual(matches[2][1], 'skip')
+    )
+  )
+
+  describe('when Swagger document has multiple responses and hooks unskip some of them', ->
+    reTransaction = /(\w+): (\w+) \/honey/g
+    matches = undefined
+
+    beforeEach((done) ->
+      execCommand(
+        options:
+          path: './test/fixtures/multiple-responses.yaml'
+          hookfiles: './test/fixtures/swagger-multiple-responses.js'
+      , (err) ->
+        matches = []
+        matches.push(groups) while groups = reTransaction.exec(stdout)
+        done(err)
+      )
+    )
+
+    it('recognizes all 3 transactions', ->
+      assert.equal(matches.length, 3)
+    )
+    it('the transaction #1 is skipped', ->
+      assert.equal(matches[0][1], 'skip')
+    )
+    it('the transaction #2 is not skipped (unskipped in hooks)', ->
+      assert.notEqual(matches[1][1], 'skip')
+    )
+    it('the transaction #3 is not skipped (status 200)', ->
+      assert.notEqual(matches[2][1], 'skip')
+    )
+  )

test/unit/transaction-runner-test.coffee

@@ -25,7 +25,7 @@ Runner = proxyquire  '../../src/transaction-runner', {
 CliReporter = require '../../src/reporters/cli-reporter'
 Hooks = require '../../src/hooks'
 
-describe 'TransactionRunner', ()->
+describe 'TransactionRunner', ->
 
   server = {}
   configuration =
@@ -98,9 +98,8 @@ describe 'TransactionRunner', ()->
 
         assert.ok runner.multiBlueprint
 
-  describe 'configureTransaction(transaction, callback)', () ->
-
-    beforeEach () ->
+  describe 'configureTransaction(transaction, callback)', ->
+    beforeEach ->
       transaction =
         name: "Machines API > Group Machine > Machine > Delete Message > Bogus example name"
         request:
@@ -125,6 +124,83 @@ describe 'TransactionRunner', ()->
 
       runner = new Runner(configuration)
 
+    describe('when processing Swagger document and given transaction has non-2xx status code', ->
+      filename = 'api-description.yml'
+      configuredTransaction = undefined
+
+      ['100', '400', 199, 300].forEach((status) ->
+        context('status code: ' + JSON.stringify(status), ->
+          beforeEach((done) ->
+            transaction.response.status = status
+            transaction.origin.filename = filename
+
+            runner.configuration.data ?= {}
+            runner.configuration.data[filename] ?= {}
+            runner.configuration.data[filename].mediaType = 'application/swagger+json'
+
+            runner.configureTransaction(transaction, (args...) ->
+              [err, configuredTransaction] = args
+              done(err)
+            )
+          )
+
+          it('skips the transaction by default', ->
+            assert.isTrue(configuredTransaction.skip)
+          )
+        )
+      )
+    )
+
+    describe('when processing Swagger document and given transaction has 2xx status code', ->
+      filename = 'api-description.yml'
+      configuredTransaction = undefined
+
+      ['200', 299].forEach((status) ->
+        context('status code: ' + JSON.stringify(status), ->
+          beforeEach((done) ->
+            transaction.response.status = status
+            transaction.origin.filename = filename
+
+            runner.configuration.data ?= {}
+            runner.configuration.data[filename] ?= {}
+            runner.configuration.data[filename].mediaType = 'application/swagger+json'
+
+            runner.configureTransaction(transaction, (args...) ->
+              [err, configuredTransaction] = args
+              done(err)
+            )
+          )
+
+          it('does not skip the transaction by default', ->
+            assert.isFalse(configuredTransaction.skip)
+          )
+        )
+      )
+    )
+
+    describe('when processing other than Swagger document and given transaction has non-2xx status code', ->
+      filename = 'api-description.yml'
+      configuredTransaction = undefined
+
+      beforeEach((done) ->
+        transaction.response.status = 400
+        transaction.origin.filename = filename
+
+        runner.configuration.data ?= {}
+        runner.configuration.data[filename] ?= {}
+        runner.configuration.data[filename].mediaType = 'text/plain'
+
+        runner.configureTransaction(transaction, (args...) ->
+          [err, configuredTransaction] = args
+          done(err)
+        )
+      )
+
+      it('does not skip the transaction by default', ->
+        assert.isFalse(configuredTransaction.skip)
+      )
+    )
+
     describe 'when processing multiple API description documents', () ->
       it 'should include api name in the transaction name', (done) ->
         runner.multiBlueprint = true