Updated Branches: refs/heads/master f680e95f1 -> 5b080ede3
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/99be0275 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/99be0275 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/99be0275 Branch: refs/heads/master Commit: 99be0275e128d2ec239cc43d96280d1fdf0cd816 Parents: 379c277 Author: Henrique <henrique@henrique-vb.(none)> Authored: Fri May 10 23:43:12 2013 +0200 Committer: Henrique <henrique@henrique-vb.(none)> Committed: Sat May 11 01:21:52 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/99be0275/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/99be0275/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/99be0275/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/99be0275/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/99be0275/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() {}; +
