You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by he...@apache.org on 2013/05/10 23:44:16 UTC

git commit: THRIFT-1957 NodeJS TFramedTransport and TBufferedTransport read bytes as unsigned Patch: Matthew Imrie

Updated Branches:
  refs/heads/master 379c2776e -> f680e95f1


THRIFT-1957 NodeJS TFramedTransport and TBufferedTransport read bytes as unsigned
Patch: Matthew Imrie

Add method to binary.js to properly read a byte type as
signed Changed transport.js readByte methods in
TFramedBuffer and TBufferedTransport to use binary.js
readByte method Added unit test for binary.js readByte
method to binary.test.js and changed test harness to
nodeunit.


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/f680e95f
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/f680e95f
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/f680e95f

Branch: refs/heads/master
Commit: f680e95f19f54fa287c56668d215448489352921
Parents: 379c277
Author: Henrique <henrique@henrique-vb.(none)>
Authored: Fri May 10 23:43:12 2013 +0200
Committer: Henrique <henrique@henrique-vb.(none)>
Committed: Fri May 10 23:43:12 2013 +0200

----------------------------------------------------------------------
 lib/nodejs/lib/thrift/binary.js    |    4 +
 lib/nodejs/lib/thrift/transport.js |    4 +-
 lib/nodejs/test/binary.test.js     |  127 ++++++++++++++++++-------------
 test/nodejs/Makefile.am            |    8 ++
 test/nodejs/client.js              |  106 +++++++++++---------------
 5 files changed, 130 insertions(+), 119 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/f680e95f/lib/nodejs/lib/thrift/binary.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/binary.js b/lib/nodejs/lib/thrift/binary.js
index 3be4d01..62c3f72 100644
--- a/lib/nodejs/lib/thrift/binary.js
+++ b/lib/nodejs/lib/thrift/binary.js
@@ -26,6 +26,10 @@ var POW_48 = Math.pow(2, 48)
 var POW_52 = Math.pow(2, 52)
 var POW_1022 = Math.pow(2, 1022)
 
+exports.readByte = function(byte){
+	return byte > 127 ? byte-256 : byte;
+}
+
 exports.readI16 = function(buff, off) {
   off = off || 0;
   var v = buff[off + 1];

http://git-wip-us.apache.org/repos/asf/thrift/blob/f680e95f/lib/nodejs/lib/thrift/transport.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/transport.js b/lib/nodejs/lib/thrift/transport.js
index 7d83a99..e918335 100644
--- a/lib/nodejs/lib/thrift/transport.js
+++ b/lib/nodejs/lib/thrift/transport.js
@@ -99,7 +99,7 @@ TFramedTransport.prototype = {
   },
 
   readByte: function() {
-    return this.inBuf[this.readPos++];
+    return binary.readByte(this.inBuf[this.readPos++]);
   },
 
   readI16: function() {
@@ -223,7 +223,7 @@ TBufferedTransport.prototype = {
 
   readByte: function() {
     this.ensureAvailable(1)
-    return this.inBuf[this.readCursor++];
+    return binary.readByte(this.inBuf[this.readCursor++]);
   },
 
   readI16: function() {

http://git-wip-us.apache.org/repos/asf/thrift/blob/f680e95f/lib/nodejs/test/binary.test.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/test/binary.test.js b/lib/nodejs/test/binary.test.js
index c6bf7c9..c7f6f72 100644
--- a/lib/nodejs/test/binary.test.js
+++ b/lib/nodejs/test/binary.test.js
@@ -17,98 +17,117 @@
  * under the License.
  */
 
-var assert = require('assert');
+var testCase = require('nodeunit').testCase;
 var binary = require('thrift/binary');
 
-module.exports = {
-  "Should read I16": function() {
-    assert.equal(0, binary.readI16([0x00, 0x00]));
-    assert.equal(1, binary.readI16([0x00, 0x01]));
-    assert.equal(-1, binary.readI16([0xff, 0xff]));
+module.exports = testCase({
+  "Should read signed byte": function(test){
+    test.equal(1, binary.readByte([0x01]));
+    test.equal(-1, binary.readByte([0xFF]));
+    
+    test.equal(127, binary.readByte([0x7F]));
+    test.equal(-128, binary.readByte([0x80]));
+    test.done();
+  },
+  "Should write byte": function(test){
+  	//Protocol simply writes to the buffer. Nothing to test.. yet.
+  	test.ok(true);
+  	test.done();
+  },
+  "Should read I16": function(test) {
+    test.equal(0, binary.readI16([0x00, 0x00]));
+    test.equal(1, binary.readI16([0x00, 0x01]));
+    test.equal(-1, binary.readI16([0xff, 0xff]));
 
     // Min I16
-    assert.equal(-32768, binary.readI16([0x80, 0x00]));
+    test.equal(-32768, binary.readI16([0x80, 0x00]));
     // Max I16
-    assert.equal(32767, binary.readI16([0x7f, 0xff]));
+    test.equal(32767, binary.readI16([0x7f, 0xff]));
+    test.done();
   },
 
-  "Should write I16": function() {
-    assert.deepEqual([0x00, 0x00], binary.writeI16([], 0));
-    assert.deepEqual([0x00, 0x01], binary.writeI16([], 1));
-    assert.deepEqual([0xff, 0xff], binary.writeI16([], -1));
+  "Should write I16": function(test) {
+    test.deepEqual([0x00, 0x00], binary.writeI16([], 0));
+    test.deepEqual([0x00, 0x01], binary.writeI16([], 1));
+    test.deepEqual([0xff, 0xff], binary.writeI16([], -1));
 
     // Min I16
-    assert.deepEqual([0x80, 0x00], binary.writeI16([], -32768));
+    test.deepEqual([0x80, 0x00], binary.writeI16([], -32768));
     // Max I16
-    assert.deepEqual([0x7f, 0xff], binary.writeI16([], 32767));
+    test.deepEqual([0x7f, 0xff], binary.writeI16([], 32767));
+    test.done();
   },
 
-  "Should read I32": function() {
-    assert.equal(0, binary.readI32([0x00, 0x00, 0x00, 0x00]));
-    assert.equal(1, binary.readI32([0x00, 0x00, 0x00, 0x01]));
-    assert.equal(-1, binary.readI32([0xff, 0xff, 0xff, 0xff]));
+  "Should read I32": function(test) {
+    test.equal(0, binary.readI32([0x00, 0x00, 0x00, 0x00]));
+    test.equal(1, binary.readI32([0x00, 0x00, 0x00, 0x01]));
+    test.equal(-1, binary.readI32([0xff, 0xff, 0xff, 0xff]));
 
     // Min I32
-    assert.equal(-2147483648, binary.readI32([0x80, 0x00, 0x00, 0x00]));
+    test.equal(-2147483648, binary.readI32([0x80, 0x00, 0x00, 0x00]));
     // Max I32
-    assert.equal(2147483647, binary.readI32([0x7f, 0xff, 0xff, 0xff]));
+    test.equal(2147483647, binary.readI32([0x7f, 0xff, 0xff, 0xff]));
+    test.done();
   },
 
-  "Should write I32": function() {
-    assert.deepEqual([0x00, 0x00, 0x00, 0x00], binary.writeI32([], 0));
-    assert.deepEqual([0x00, 0x00, 0x00, 0x01], binary.writeI32([], 1));
-    assert.deepEqual([0xff, 0xff, 0xff, 0xff], binary.writeI32([], -1));
+  "Should write I32": function(test) {
+    test.deepEqual([0x00, 0x00, 0x00, 0x00], binary.writeI32([], 0));
+    test.deepEqual([0x00, 0x00, 0x00, 0x01], binary.writeI32([], 1));
+    test.deepEqual([0xff, 0xff, 0xff, 0xff], binary.writeI32([], -1));
 
     // Min I32
-    assert.deepEqual([0x80, 0x00, 0x00, 0x00], binary.writeI32([], -2147483648));
+    test.deepEqual([0x80, 0x00, 0x00, 0x00], binary.writeI32([], -2147483648));
     // Max I32
-    assert.deepEqual([0x7f, 0xff, 0xff, 0xff], binary.writeI32([], 2147483647));
+    test.deepEqual([0x7f, 0xff, 0xff, 0xff], binary.writeI32([], 2147483647));
+ 	test.done();
   },
 
-  "Should read doubles": function() {
-    assert.equal(0, binary.readDouble([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
-    assert.equal(0, binary.readDouble([0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
-    assert.equal(1, binary.readDouble([0x3f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
-    assert.equal(2, binary.readDouble([0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
-    assert.equal(-2, binary.readDouble([0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+  "Should read doubles": function(test) {
+    test.equal(0, binary.readDouble([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(0, binary.readDouble([0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(1, binary.readDouble([0x3f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(2, binary.readDouble([0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(-2, binary.readDouble([0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
 
-    assert.equal(Math.PI, binary.readDouble([0x40, 0x9, 0x21, 0xfb, 0x54, 0x44, 0x2d, 0x18]))
+    test.equal(Math.PI, binary.readDouble([0x40, 0x9, 0x21, 0xfb, 0x54, 0x44, 0x2d, 0x18]))
 
-    assert.equal(Infinity, binary.readDouble([0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
-    assert.equal(-Infinity, binary.readDouble([0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(Infinity, binary.readDouble([0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(-Infinity, binary.readDouble([0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
 
-    assert.ok(isNaN(binary.readDouble([0x7f, 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00])))
+    test.ok(isNaN(binary.readDouble([0x7f, 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00])))
 
-    assert.equal(1/3, binary.readDouble([0x3f, 0xd5, 0x55, 0x55, 0x55, 0x55, 0x55, 0x55]))
+    test.equal(1/3, binary.readDouble([0x3f, 0xd5, 0x55, 0x55, 0x55, 0x55, 0x55, 0x55]))
 
     // Min subnormal positive double
-    assert.equal(4.9406564584124654e-324, binary.readDouble([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01]))
+    test.equal(4.9406564584124654e-324, binary.readDouble([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01]))
     // Min normal positive double
-    assert.equal(2.2250738585072014e-308, binary.readDouble([0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
+    test.equal(2.2250738585072014e-308, binary.readDouble([0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]))
     // Max positive double
-    assert.equal(1.7976931348623157e308, binary.readDouble([0x7f, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]))
+    test.equal(1.7976931348623157e308, binary.readDouble([0x7f, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]))
+  	test.done();
   },
 
-  "Should write doubles": function() {
-    assert.deepEqual([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 0));
-    assert.deepEqual([0x3f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 1));
-    assert.deepEqual([0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 2));
-    assert.deepEqual([0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], -2));
+  "Should write doubles": function(test) {
+    test.deepEqual([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 0));
+    test.deepEqual([0x3f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 1));
+    test.deepEqual([0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 2));
+    test.deepEqual([0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], -2));
 
-    assert.deepEqual([0x40, 0x9, 0x21, 0xfb, 0x54, 0x44, 0x2d, 0x18], binary.writeDouble([], Math.PI));
+    test.deepEqual([0x40, 0x9, 0x21, 0xfb, 0x54, 0x44, 0x2d, 0x18], binary.writeDouble([], Math.PI));
 
-    assert.deepEqual([0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], Infinity));
-    assert.deepEqual([0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], -Infinity));
+    test.deepEqual([0x7f, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], Infinity));
+    test.deepEqual([0xff, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], -Infinity));
 
-    assert.deepEqual([0x7f, 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], NaN));
+    test.deepEqual([0x7f, 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], NaN));
 
-    assert.deepEqual([0x3f, 0xd5, 0x55, 0x55, 0x55, 0x55, 0x55, 0x55], binary.writeDouble([], 1/3));
+    test.deepEqual([0x3f, 0xd5, 0x55, 0x55, 0x55, 0x55, 0x55, 0x55], binary.writeDouble([], 1/3));
 
     // Min subnormal positive double
-    assert.deepEqual([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01], binary.writeDouble([], 4.9406564584124654e-324)); 
+    test.deepEqual([0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01], binary.writeDouble([], 4.9406564584124654e-324)); 
     // Min normal positive double
-    assert.deepEqual([0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 2.2250738585072014e-308)); 
+    test.deepEqual([0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], binary.writeDouble([], 2.2250738585072014e-308)); 
     // Max positive double
-    assert.deepEqual([0x7f, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff], binary.writeDouble([], 1.7976931348623157e308)); 
+    test.deepEqual([0x7f, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff], binary.writeDouble([], 1.7976931348623157e308)); 
+  	test.done();
   }
-}
+});
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/thrift/blob/f680e95f/test/nodejs/Makefile.am
----------------------------------------------------------------------
diff --git a/test/nodejs/Makefile.am b/test/nodejs/Makefile.am
index 32397f1..8f74ba8 100755
--- a/test/nodejs/Makefile.am
+++ b/test/nodejs/Makefile.am
@@ -22,6 +22,14 @@ stubs: ../ThriftTest.thrift
 	$(THRIFT) --gen js:node ../ThriftTest.thrift
 
 check: stubs
+	@if which expresso &> /dev/null ; then \
+		echo "   Testing thrift/binary"; \
+		NODE_PATH=../../lib/nodejs/lib:../../lib/nodejs/lib/thrift:$(NODE_PATH) nodeunit ../../lib/nodejs/test/binary.test.js; \
+	fi
+	timeout 2 $(MAKE) server &
+	@sleep 1
+	$(MAKE) client
+	@sleep 1
 
 clean-local:
 	$(RM) -r gen-nodejs

http://git-wip-us.apache.org/repos/asf/thrift/blob/f680e95f/test/nodejs/client.js
----------------------------------------------------------------------
diff --git a/test/nodejs/client.js b/test/nodejs/client.js
index 88460b0..3a90cac 100644
--- a/test/nodejs/client.js
+++ b/test/nodejs/client.js
@@ -17,6 +17,7 @@
  * under the License.
  */
 var thrift = require('thrift');
+var assert = require('assert');
 
 var ThriftTest = require('./gen-nodejs/ThriftTest'),
     ttypes = require('./gen-nodejs/ThriftTest_types');
@@ -24,81 +25,61 @@ var ThriftTest = require('./gen-nodejs/ThriftTest'),
 var connection = thrift.createConnection('localhost', 9090),
     client = thrift.createClient(ThriftTest, connection);
 
-var tfailed = 0;
-var tpassed = 0;
-
-function failed(err) {
-  console.trace(err);
-	return tfailed++;
-}
-function passed() {
-	return tpassed++;
-}
-
 connection.on('error', function(err) {
-  failed(err);
+  assert(false, err);
 });
 
-console.time("Tests completed in");
+
 
 client.testVoid(function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testVoid() = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(undefined, response);
 });
 
 client.testString("Test", function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testString('Test') = ", response);
-	passed();
+  assert( ! err);
+  assert.equal("Test", response);
 });
 
 client.testByte(1, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testByte(1) = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(1, response);
 });
 
 client.testI32(-1, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testI32(-1) = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(-1, response);
 });
 
 client.testI64(5, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testI64(5) = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(5, response);
 });
 
 client.testI64(-5, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testI64(-5) = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(-5, response);
 });
 
 client.testI64(-34359738368, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testI64(-34359738368) = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(-34359738368, response);
 });
 
 client.testDouble(-5.2098523, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testDouble(-5.2098523) = ", response);
-	passed();
+  assert( ! err);
+  assert.equal(-5.2098523, response);
 });
 
 var out = new ttypes.Xtruct({
-	string_thing: 'Zero',
-	byte_thing: 1,
-	i32_thing: -3,
-	i64_thing: 1000000
+  string_thing: 'Zero',
+  byte_thing: 1,
+  i32_thing: -3,
+  //i64_thing: 1000000
 });
 client.testStruct(out, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testStruct(", out, ") = \n", response);
-	passed();
+  assert( ! err);
+  assert.deepEqual(out, response);
 });
 
 var out2 = new ttypes.Xtruct2();
@@ -106,9 +87,8 @@ out2.byte_thing = 1;
 out2.struct_thing = out;
 out2.i32_thing = 5;
 client.testNest(out2, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testNest(", out2, ") = \n", response);
-	passed();
+  assert( ! err);
+  assert.deepEqual(out2, response);
 });
 
 var mapout = {};
@@ -116,9 +96,8 @@ for (var i = 0; i < 5; ++i) {
   mapout[i] = i-10;
 }
 client.testMap(mapout, function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testMap(", mapout, ") = \n", response);
-	passed();
+  assert( ! err);
+  assert.deepEqual(mapout, response);
 });
 
 /*
@@ -127,25 +106,26 @@ client.testMap(mapout, function(err, response) {
 
 
 client.testException('ApplicationException', function(err, response) {
-  console.log("testException('ApplicationException') = ", err);
-  if (response) { return failed(response); }
-	passed();
+  //assert.equal('ApplicationException', err);
+  assert( ! response);
 });
 
 client.testException('Xception', function(err, response) {
-  console.log("testException('Xception') = ", err);
-  if (response) { return failed(response); }
-	passed();
+  assert.equal('Xception', err.message);
+  assert( ! response);
 });
 
 client.testException('success', function(err, response) {
-  if (err) { return failed(err); }
-  console.log("testException('success') = ", response);
-	passed();
+  assert( ! err);
+  //assert.equal('success', response);
 });
 
-setTimeout(function(){
-  console.timeEnd("Tests completed in");
-	console.log(tpassed + " passed, " + tfailed + " failed");
-	connection.end();
+
+setTimeout(function() {
+  console.log("Server successfully tested!");
+  connection.end();
 }, 200);
+
+// to make it also run on expresso
+exports.expressoTest = function() {};
+