attachments: remove intermediate conversion to string

Author: charlierudolph

features/attachments.feature

@@ -19,7 +19,7 @@ Feature: Attachments
       """
       var hooks = function () {
         this.Before(function(scenario, callback) {
-          scenario.attach(new Buffer([100, 97, 116, 97]), 'image/png');
+          scenario.attach(new Buffer([137, 80, 78, 71]), 'image/png');
           callback();
         });
       };
@@ -60,7 +60,7 @@ Feature: Attachments
                   "embeddings": [
                     {
                       "mime_type": "image/png",
-                      "data": "ZGF0YQ=="
+                      "data": "iVBORw=="
                     }
                   ]
                 },
@@ -101,27 +101,19 @@ Feature: Attachments
       """
     And a file named "features/support/hooks.js" with:
       """
+      var Stream = require('stream');
+
       var hooks = function () {
         this.Before(function(scenario, callback) {
-          var Stream = require('stream');
-          var versionParts = /v(\d+)\.(\d+)\.(\d+)/.exec(process.version);
-          var major = parseInt(versionParts[0], 10);
-          var minor = parseInt(versionParts[1], 10);
-
-          if (major > 0 || minor >= 10) {
-            var stream = new Stream.Readable();
-            stream._read = function() {};
-            stream.push(new Buffer([100, 97, 116, 97]));
-            stream.push(null);
+          var stream = new Stream.Readable();
+          stream._read = function() {};
+          stream.push(new Buffer([137, 80]));
+          stream.push(new Buffer([78, 71]));
+          stream.push(null);
 
-            scenario.attach(stream, 'image/png', function(error) {
-              callback(error);
-            });
-          }
-          else {
-            scenario.attach(new Buffer([100, 97, 116, 97]), 'image/png');
-            callback();
-          }
+          scenario.attach(stream, 'image/png', function(error) {
+            callback(error);
+          });
         });
       };
 
@@ -156,12 +148,12 @@ Feature: Attachments
                   },
                   "arguments": [],
                   "match": {
-                    "location": "<current-directory>/features/support/hooks.js:2"
+                    "location": "<current-directory>/features/support/hooks.js:4"
                   },
                   "embeddings": [
                     {
                       "mime_type": "image/png",
-                      "data": "ZGF0YQ=="
+                      "data": "iVBORw=="
                     }
                   ]
                 },

lib/cucumber/api/scenario.js

@@ -7,8 +7,8 @@ function Scenario(astScenario, scenarioResult) {
     return value && typeof value === 'object' && typeof value.pipe === 'function';
   }
 
-  function attachString(string, mimeType) {
-    var attachment = Cucumber.Runtime.Attachment({mimeType: mimeType, data: string});
+  function attachData(data, mimeType) {
+    var attachment = Cucumber.Runtime.Attachment({mimeType: mimeType, data: data});
     attachments.push(attachment);
   }
 
@@ -19,16 +19,11 @@ function Scenario(astScenario, scenarioResult) {
       buffers.push(chunk);
     });
     stream.on('end', function () {
-      attachString(Buffer.concat(buffers).toString(), mimeType);
+      attachData(Buffer.concat(buffers), mimeType);
       callback();
     });
   }
 
-  function attachBuffer(buffer, mimeType, callback) {
-    attachString(buffer.toString(), mimeType);
-    if (callback) callback();
-  }
-
   var self = {
     getKeyword:     function getKeyword()     { return astScenario.getKeyword(); },
     getName:        function getName()        { return astScenario.getName(); },
@@ -58,14 +53,17 @@ function Scenario(astScenario, scenarioResult) {
         if (!mimeType)
           throw Error(Scenario.ATTACH_MISSING_MIME_TYPE_ARGUMENT);
 
-        attachBuffer(data, mimeType, callback);
+        attachData(data, mimeType);
+        if (callback) callback();
       }
-      else {
+      else if (typeof(data) === 'string') {
         if (!mimeType)
           mimeType = Scenario.DEFAULT_TEXT_MIME_TYPE;
 
-        attachString(data.toString(), mimeType);
+        attachData(data, mimeType);
         if (callback) callback();
+      } else {
+        throw Error(Scenario.ATTACH_INVALID_DATA_TYPE);
       }
     }
   };
@@ -76,5 +74,6 @@ function Scenario(astScenario, scenarioResult) {
 Scenario.DEFAULT_TEXT_MIME_TYPE = 'text/plain';
 Scenario.ATTACH_MISSING_MIME_TYPE_ARGUMENT = 'Cucumber.Api.Scenario.attach() expects a mimeType';
 Scenario.ATTACH_MISSING_CALLBACK_ARGUMENT_FOR_STREAM_READABLE = 'Cucumber.Api.Scenario.attach() expects a callback when data is a stream.Readable';
+Scenario.ATTACH_INVALID_DATA_TYPE = 'Cucumber.Api.Scenario.attach() expects data to be a stream, buffer, or string.';
 
 module.exports = Scenario;

spec/cucumber/api/scenario_spec.js

@@ -217,7 +217,7 @@ describe("Cucumber.Api.Scenario", function () {
             var attachments = scenario.getAttachments();
             expect(attachments.length).toEqual(1);
             var attachment = attachments[0];
-            expect(attachment.getData()).toEqual("first chunksecond chunk");
+            expect(attachment.getData().toString()).toEqual("first chunksecond chunk");
             expect(attachment.getMimeType()).toEqual(mimeType);
           });
 
@@ -244,7 +244,7 @@ describe("Cucumber.Api.Scenario", function () {
         var attachments = scenario.getAttachments();
         expect(attachments.length).toEqual(1);
         var attachment = attachments[0];
-        expect(attachment.getData()).toEqual("data");
+        expect(attachment.getData().toString()).toEqual("data");
         expect(attachment.getMimeType()).toEqual(mimeType);
       });
 
@@ -300,5 +300,19 @@ describe("Cucumber.Api.Scenario", function () {
         expect(callback).toHaveBeenCalled();
       });
     });
+
+    describe("when the data is an object", function () {
+      var data;
+
+      beforeEach(function () {
+        data = {};
+      });
+
+      it("throws an error", function () {
+        expect(function() {
+          scenario.attach(data, mimeType);
+        }).toThrow(new Error("Cucumber.Api.Scenario.attach() expects data to be a stream, buffer, or string."));
+      });
+    });
   });
 });