pierrec / node-lz4

LZ4 fast compression algorithm for NodeJS
MIT License
438 stars 98 forks source link

Added karma to run tests in Chrome for encoding & decoding #14

Closed tpodom closed 10 years ago

tpodom commented 10 years ago

Added karma tests to validate browser functionality.

I'm currently running into two issues with the tests.

My main issue is that decoding the sphere.dae fails with a stream checksum error. I tested the data/test.lz4 and it worked. I'm not sure if that's because it was created without a stream checksum? I'm not really sure where to begin looking as it appears to get through all of the blocks and then fails on the final checksum. I am able to decode the sphere.lz4 file from a node script though so it's definitely something different between node & browser.

The second is issue #13 which prevents any encoding in the browser (short or long text).

The sphere.lz4.dat file (named .dat so karma serves it as binary) was created with the following node script:

var fs = require('fs')
var lz4 = require('lz4')
var bomstrip = require('bomstrip');
var encoder = lz4.createEncoderStream()

var input = fs.createReadStream(process.argv[2], {encoding: 'utf8'})
var output = fs.createWriteStream(process.argv[2] + '.lz4')
input.pipe(new bomstrip()).pipe(encoder).pipe(output)
pierrec commented 10 years ago

Thanks for the karma tests, very helpful indeed. I think I have finally nailed down the issue: the browser implementation of Buffer does do sign checks in writeUint32LE(), which doesnt make sense to me. I have changed the code to use writeInt32LE() in places where it can have a sign (checkum being one of them, hence your checksum errors). The karma tests do pass on my system now.

Let me know if this works for you.

tpodom commented 10 years ago

The encoding is definitely working but I'm still getting stream checksum failures: Chrome 35.0.1916 (Mac OS X 10.9.3) Browser encoding sync should be able to encode and decode collada file FAILED Error: Invalid stream checksum: C354179F @19485

pierrec commented 10 years ago

Hmm. Sorry I missed this. Fixed as of 0.3.9.