Sync upstream test-buffer-alloc #178

vmx · 2018-01-21T01:53:38Z

This pull request updates the test-buffer-alloc.js from Node.js to the current upstream version.

I've used very granular commits which hopefully shows my workflow. I hope my approach is sound and the work was worth it.

This is part of #177.

This commits updates the `test-buffer-alloc.js`. Tests currently not passing are commented out and will be fixed in a subsequent commit.

Node.js treats an equal sign as the end of a Base64 encoded string. Hence `Buffer.from('=bad')` results in an empty string.

If a hex string has an odd number of bytes, the last byte is just ignored in Node.js.

The error message changed from "encoding" must be a valid string encoding to Unknown encoding: <the-given-encoding> To make the test work, parts of Node.js's `common` module need to be used. Add the parts that are needed with minor modifications (the check for the return code were removed, as vanilla JS errors don't have an error code, only Node.js errors have).

We don't want to print anything, hence `printSkipMessage()` is just a no-op to make the tests without changes pass.

Make `toLocaleString()` to point to the same implementation as `toString()`.

This fixed is based on Node.js' commit d4f00fe1098b0d7b8444565e741d8b457fd9c091 nodejs/node@d4f00fe

Some tests from test-buffer.js were moved into test-buffer-alloc.js. Remove those tests as they got updated (and fixed) in test-buffer-alloc.js.

Some post-processing of the downloaded tests is no longer needed. Use `const` and `let` instead of `var` and also import a trimmed down version of the `common` test helper.

This commit uses the `test-buffer-alloc.js` from upstream Node.js as of 2018-01-20. This is part of feross#177.

feross · 2018-02-16T03:42:39Z

index.js

@@ -53,6 +53,16 @@ function typedArraySupport () {
  }
 }

+Object.defineProperty(Buffer.prototype, 'offset', {
+  enumerable: true,


This should probably be enumerable: false, since we don't want these properties to show up in Object.keys(). Here's what Node.js does:

Object.keys(Buffer.from([1, 2,3])) // [ '0', '1', '2' ]

feross · 2018-02-16T03:43:17Z

index.js

@@ -53,6 +53,16 @@ function typedArraySupport () {
  }
 }

+Object.defineProperty(Buffer.prototype, 'parent', {
+  enumerable: true,


Let's remove this so the default value of enumerable: false is used. We don't want Object.keys(Buffer.from([1, 2, 3])) to return [1, 2, 3, 'offset', 'parent'].

feross · 2018-02-16T03:50:50Z

index.js

@@ -1560,9 +1560,11 @@ Buffer.prototype.fill = function fill (val, start, end, encoding) {
 // HELPER FUNCTIONS
 // ================

-var INVALID_BASE64_RE = /[^+/0-9A-Za-z-_]/g
+var INVALID_BASE64_RE = /[^+/0-9A-Za-z-_=]/g


I don't think this line needs to be changed. In the new line you added below, you're already finding the first = and discarding it and everything that follows. There should be no = left in the string to remove.

feross · 2018-02-16T04:18:43Z

index.js

@@ -1445,6 +1445,9 @@ Buffer.prototype.writeDoubleBE = function writeDoubleBE (value, offset, noAssert

 // copy(targetBuffer, targetStart=0, sourceStart=0, sourceEnd=buffer.length)
 Buffer.prototype.copy = function copy (target, targetStart, start, end) {
+  if (target === undefined) {


Better to check if the argument is not actually a Buffer to catch all possible bad values instead of just undefined

feross · 2018-02-16T04:28:55Z

index.js

+
+Object.defineProperty(Buffer.prototype, 'offset', {
+  enumerable: true,
+  get () {


This type of function declaration breaks IE11, fortunately we can just use get: function () {

feross · 2018-02-16T04:43:37Z

test/common.js

+
+  const failed = mustCallChecks.filter(function(context) {
+    if ('minimum' in context) {
+      context.messageSegment = `at least ${context.minimum}`;


The template literal syntax breaks IE11.

feross · 2018-02-16T04:46:12Z

test/common.js

+  return _mustCallInner(fn, exact, 'exact');
+};
+
+function _mustCallInner(fn, criteria = 1, field) {


Default argument values break IE11

feross · 2018-02-16T05:55:20Z

I figured out the reason that IE11 was unhappy -- test/common.js belongs in test/node/common.js. The files that match test/* run in both ES5 and ES6+ browsers, while test/node/* only run in ES6+ browsers. Moving the file into node/ fixes the IE11 errors.

feross · 2018-02-16T05:55:51Z

Fixed the other issues I mentioned and merged this. Thanks for doing this work!

vmx added 21 commits January 18, 2018 22:55

test-buffer-alloc updated from uptream

4fd1350

This commits updates the `test-buffer-alloc.js`. Tests currently not passing are commented out and will be fixed in a subsequent commit.

Treat = as end of Base64 encoded string

f15c393

Node.js treats an equal sign as the end of a Base64 encoded string. Hence `Buffer.from('=bad')` results in an empty string.

Hex strings no longer need have even number of digits

68c2a6e

If a hex string has an odd number of bytes, the last byte is just ignored in Node.js.

Add printSkipMessage() stub

34d49a3

We don't want to print anything, hence `printSkipMessage()` is just a no-op to make the tests without changes pass.

Throw error when no buffer is given for copy()

c404fd1

Error message on Buffer creation changed

87fd466

Out of bounds error message changed

cb98971

Out of bounds error message changed

66ae1ae

Node.js also supports Buffer creation from {buffer: ArrayBuffer}

cf6061e

Error message for "size" parameter changed

e39a513

Custom implementation of toLocaleString()

25f8d99

Make `toLocaleString()` to point to the same implementation as `toString()`.

Throw error on alloc() if value can't be encoded

3cc7458

Fix single character fast path

df827fb

This fixed is based on Node.js' commit d4f00fe1098b0d7b8444565e741d8b457fd9c091 nodejs/node@d4f00fe

Add .offset property

7d18659

const is needed to fix some test

fb249b8

Add .parent property

63a0e8b

Fix outdated error message

386e11a

Remove tests that were moved

2c7d6f3

Some tests from test-buffer.js were moved into test-buffer-alloc.js. Remove those tests as they got updated (and fixed) in test-buffer-alloc.js.

Remove some post-processing from download script

06aab7e

Some post-processing of the downloaded tests is no longer needed. Use `const` and `let` instead of `var` and also import a trimmed down version of the `common` test helper.

Use automatically downloaded test-buffer-alloc

d1bbce0

This commit uses the `test-buffer-alloc.js` from upstream Node.js as of 2018-01-20. This is part of feross#177.

vmx force-pushed the sync-tests-alloc branch from f1b39ed to d1bbce0 Compare January 21, 2018 02:07

This was referenced Feb 10, 2018

Buffer shim is no longer working in the Browser as expected ipfs-inactive/js-ipfs-http-client#651

Merged

Issue with how Fixtures get loaded ipfs-inactive/js-ipfs-http-client#689

Merged

feross reviewed Feb 16, 2018

View reviewed changes

feross merged commit d1bbce0 into feross:master Feb 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Sync upstream test-buffer-alloc #178

Sync upstream test-buffer-alloc #178

Uh oh!

vmx commented Jan 21, 2018

Uh oh!

feross Feb 16, 2018

Uh oh!

feross Feb 16, 2018 •

edited

Loading

Uh oh!

feross Feb 16, 2018

Uh oh!

feross Feb 16, 2018

Uh oh!

feross Feb 16, 2018

Uh oh!

feross Feb 16, 2018

Uh oh!

feross Feb 16, 2018

Uh oh!

feross commented Feb 16, 2018

Uh oh!

feross commented Feb 16, 2018

Uh oh!

Uh oh!

Uh oh!

Sync upstream test-buffer-alloc #178

Sync upstream test-buffer-alloc #178

Uh oh!

Conversation

vmx commented Jan 21, 2018

Uh oh!

feross Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

feross Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feross Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

feross Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

feross Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

feross Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

feross Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

feross commented Feb 16, 2018

Uh oh!

feross commented Feb 16, 2018

Uh oh!

Uh oh!

feross Feb 16, 2018 •

edited

Loading