Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node.js simple tests #1

Open
sokra opened this issue Jun 6, 2012 · 13 comments
Open

node.js simple tests #1

sokra opened this issue Jun 6, 2012 · 13 comments

Comments

@sokra
Copy link
Contributor

sokra commented Jun 6, 2012

I tried to run this module with the Buffer tests from node
( https://github.com/joyent/node/blob/master/test/simple/test-buffer.js )

Tests run in Chrome 19.0.1084.52 m. May not all tests can be fixed...

I tried to omit duplicate errors.

This are the results:

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L33

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L48

TypeError: Object #<error> has no method 'copy'
    at Buffer.copy (buffer-browserify/index.js:598:22)

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L186

AssertionError: "hello world" == "\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000"

writing and reading via buffer[number] does not work. This causes that error. (and that in L207)

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L250

Error: Unknown encoding
    at Function.byteLength (buffer-browserify/index.js:58:13)
    at new Buffer (buffer-browserify/index.js:323:30)

Encodings: 'ucs2', 'utf-16le', 'binary'

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L320

the base64 decoder should ignores whitespace and illegal chars.

Invalid string. Length must be a multiple of 4

https://github.com/beatgammit/base64-js/blob/master/lib/b64.js#L10 should throw an Error not a string!

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L343

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L379 and more

Invalid string. Length must be a multiple of 4

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L432

TypeError: Object #<error> has no method 'binarySlice'
    at Buffer.toString (buffer-browserify/index.js:512:26)

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L569

TypeError: Object #<error> has no method 'fill'
    at Buffer.fill (buffer-browserify/index.js:558:22)

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L605

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L623

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L706

https://github.com/joyent/node/blob/master/test/simple/test-buffer.js#L719

@toots
Copy link
Owner

toots commented Jun 6, 2012

Indeed, not all functionalities from node are covered yet. Will add them whenever possible. Feel free to submit a PR too :-)

@sokra
Copy link
Contributor Author

sokra commented Jun 6, 2012

no hurry. I just wanted to document it.

@ysangkok
Copy link
Contributor

Is this report still current or has the library been superseded or something?

@toots
Copy link
Owner

toots commented Sep 19, 2012

The report is still current. The library is functional as it is but does not implement all functionalities from node yet.

@GraemeF
Copy link

GraemeF commented Oct 21, 2012

As a relative JS noob I'm wondering if it's even possible to fix writing and reading via buffer[number]? I'm trying to use https://github.com/creationix/jsonparse with browserify and this is making it barf. If it can't be fixed here then I'll have to look at maybe detecting browserify and using something other than a Buffer in those cases.

@ysangkok
Copy link
Contributor

@GraemeF
Copy link

GraemeF commented Oct 21, 2012

@ysangkok : Ah, just as I feared. Thanks!

@rdrey
Copy link
Contributor

rdrey commented Nov 15, 2012

Ah, I just got bitten by the buf[i] issue, too. Luckily the call is in my own code, so it's simple to replace it with buf.get(i) for now.

@toots
Copy link
Owner

toots commented Mar 9, 2013

I'm all about fixing the buf[i] issue. Typed array sounds promising. However, browser support for those seem pretty crappy so I don't think that this is actually a viable solution for now, unless there is some kind of browser detection+fallback which would definitely yield bad code I fear..

@alexstrat
Copy link

@toots I'm curious to know which technique you plan to use for fixing the buf[i] issue: __defineGetter__, defineProperty or others ? buf[i] = this.parent[i] does not do the hack, right ? Have you already started something ?

@GraemeF
Copy link

GraemeF commented Mar 18, 2013

FYI @substack is working on a patch for the buf[i] issue (it was mentioned in dominictarr/JSONStream#23 (comment)).

@toots
Copy link
Owner

toots commented Mar 18, 2013

Hi all,

@alexstrat I have no preference as to which technique to use, my only concerns are:

  • Compatibility with a reasonable set of browsers
  • Backward compatibility with node.js' original buffers (including potential boog/weird API specifics)

I have not yet started working on it but, @GraemeF : any patch for this would be much welcome and I'd try to review and merge in a timely fashion, should that happen! :-)

@ghost
Copy link

ghost commented Mar 20, 2013

Here's the buf[i] patch: #14
It's really big because it unfortunately required merging SlowBuffer with Buffer because buf[i] only works with SlowBuffer but the external interface differs from Buffer. However, all the old tests pass plus some new ones to test buf[i].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants