Timers should work even if the user has monkey-patched `.call()` and
`.apply()` to undesirable values.
PR-URL: https://github.com/nodejs/node/pull/12960
Ref: https://github.com/nodejs/node/issues/12956
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
raise(SIGABRT) or CRT abort causes exit code 3 and
null signal in windows. Looks like this simple assertion
is not present in windows. Make this assertion.
PR-URL: https://github.com/nodejs/node/pull/12914
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove common.PORT from test-https-connect-address-family to eliminate
possibility that a dynamic port used in another test will collide with
common.PORT.
PR-URL: https://github.com/nodejs/node/pull/12915
Ref: https://github.com/nodejs/node/issues/12376
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
parallel/test-spawn-cmd-named-pipe.js failed with spaces
both in node.exe and test paths.
PR-URL: https://github.com/nodejs/node/pull/12945
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently when configured --without-ssl these test will fail. In
test-crypto-pbkdf2.js and test-crypto-randomBytes.js the check exists
but just need to be moved before the require of crypto.
There was no check in test-async-wrap-uncaughtexception.js so one was
added.
PR-URL: https://github.com/nodejs/node/pull/12968
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Luca Maraschi <luca.maraschi@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit replaces a fprintf() with a call to
ProcessEmitWarning().
Refs: https://github.com/nodejs/node/issues/12706
PR-URL: https://github.com/nodejs/node/pull/12709
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
test-http-mutable-headers uses assert.deepEqual() in three places but
appears to only needs it in two of them. Replace one with
assert.deepStrictEqual() and remove linting exception.
PR-URL: https://github.com/nodejs/node/pull/12883
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Removed common.PORT from test-cluster-disconnect to eliminate the
possibility that a port used in another test will collide
with common.PORT.
PR-URL: https://github.com/nodejs/node/pull/12545
Refs: https://github.com/nodejs/node/issues/12376
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Fill this commit messsage with more details about the change once all
changes are rebased.
* Add lib/async_hooks.js
* Add JS methods to AsyncWrap for handling the async id stack
* Introduce AsyncReset() so that JS functions can reset the id and again
trigger the init hooks, allow AsyncWrap::Reset() to be called from JS
via asyncReset().
* Add env variable to test additional things in test/common.js
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Allow handles to retrieve their own uid's by adding a new method on the
FunctionTemplates. Implementation of these into all other classes will
come in a future commit.
Add the method AsyncWrap::GetAsyncId() to all inheriting class objects
so the uid of the handle can be retrieved from JS.
In all applicable locations, run ClearWrap() on the object holding the
pointer so that it never points to invalid memory and make sure Wrap()
is always run so the class pointer is correctly attached to the object
and can be retrieved so GetAsyncId() can be run.
In many places a class instance was not removing its own pointer from
object() in the destructor. This left an invalid pointer in the JS
object that could cause the application to segfault under certain
conditions.
Remove ClearWrap() from ReqWrap for continuity. The ReqWrap constructor
was not the one to call Wrap(), so it shouldn't be the one to call
ClearWrap().
Wrap() has been added to all constructors that inherit from AsyncWrap.
Normally it's the child most class. Except in the case of HandleWrap.
Which must be the constructor that runs Wrap() because the class pointer
is retrieved for certain calls and because other child classes have
multiple inheritance to pointer to the HandleWrap needs to be stored.
ClearWrap() has been placed in all FunctionTemplate constructors so that
no random values are returned when running getAsyncId(). ClearWrap() has
also been placed in all class destructors, except in those that use
MakeWeak() because the destructor will run during GC. Making the
object() inaccessible.
It could be simplified to where AsyncWrap sets the internal pointer,
then if an inheriting class needs one of it's own it could set it again.
But the inverse would need to be true also, where AsyncWrap then also
runs ClearWeak. Unforunately because some of the handles are cleaned up
during GC that's impossible. Also in the case of ReqWrap it runs Reset()
in the destructor, making the object() inaccessible. Meaning,
ClearWrap() must be run by the class that runs Wrap(). There's currently
no generalized way of taking care of this across all instances of
AsyncWrap.
I'd prefer that there be checks in there for these things, but haven't
found a way to place them that wouldn't be just as unreliable.
Add test that checks all resources that can run getAsyncId(). Would like
a way to enforce that any new classes that can also run getAsyncId() are
tested, but don't have one.
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
AsyncWrap will be going through many changes, and the old API will no
longer be used. So remove those tests that will no longer be useful.
They may be added back later using the new API, once fully implemented.
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Also add checks in lib/tty.js and tests.
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Replace the `assert.throws` second argument from a Type to a `RegExp`
matching the entire error message.
Error message changes are `semver-major`, so we assert their content.
PR-URL: https://github.com/nodejs/node/pull/12891
Refs: https://github.com/nodejs/node/pull/12603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fixed test-https-client-get-url by waiting on HTTPS GET requests
to finish before closing the server.
PR-URL: https://github.com/nodejs/node/pull/12876
Fixes: https://github.com/nodejs/node/issues/12873
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
The `max-len` ESLint rule is configured to be forgiving for lines that
contain a regular expression literal. Remove disabling comments that are
no longer required in test-repl.js
PR-URL: https://github.com/nodejs/node/pull/12868
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove common.PORT from, test-net-connect-immediate-destroy,
test-net-options-lookup, test-net-connect-local-error,
test-net-connect-handle-econnrefused, test-net-socket-destroy-twice,
test-net-better-error-messages-port-hostname, test-net-localerror,
to reduce possibility that a dynamic port used in another test will
collide with common.PORT.
Moved test-net-listen-shared-ports, test-net-better-error-messages-port
from tests/parallel to test/sequential
Refs: https://github.com/nodejs/node/issues/12376
PR-URL: https://github.com/nodejs/node/pull/12473
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
If an asynchronous function is passed no callback function, there is no
way to return the result. This patch throws an error if the callback
passed is not valid or none passed at all.
PR-URL: https://github.com/nodejs/node/pull/12562
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Add support for `util.promisify(setTimeout)` and
`util.promisify(setImmediate)` as a proof-of-concept implementation.
`clearTimeout()` and `clearImmediate()` do not work on those Promises.
Includes documentation and tests.
PR-URL: https://github.com/nodejs/node/pull/12442
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: William Kapke <william.kapke@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Take tests from Bluebird's promisify's tests and adapted them to the
format in use here.
Add tests making sure things work with async functions.
Add basic usability tests.
PR-URL: https://github.com/nodejs/node/pull/12442
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: William Kapke <william.kapke@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Add methods for creating, resolving and rejecting promises
using the V8 C++ API that does not require creation of extra
`resolve` and `reject` functions to `process.binding('util')`.
PR-URL: https://github.com/nodejs/node/pull/12442
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: William Kapke <william.kapke@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Currently this test will fail if node was configured --without-ssl.
PR-URL: https://github.com/nodejs/node/pull/12882
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This commit attempts fix a TODO in net.js:
TODO(bnoordhuis) Check err and throw?
PR-URL: https://github.com/nodejs/node/pull/12871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It's important for people who monkey-patch `Socket.prototype.connect`
that it's called by `net.connect` since it's not possible to
monkey-patch `net.connect` directly (as the `connect` function is called
directly by other parts of `lib/net.js` instead of calling the
`exports.connect` function).
Among the actors who monkey-patch `Socket.prototype.connect` are most
APM vendors, the async-listener module and the
continuation-local-storage module.
Related:
- https://github.com/nodejs/node/pull/12342
- https://github.com/nodejs/node/pull/12852
PR-URL: https://github.com/nodejs/node/pull/12861
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luca Maraschi <luca.maraschi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
In Node 7.x, calling `throw new assert.AssertionError()` resulted in a
TypeError.
In current master, the same call does not result in an error but, due to
the default option, it results in uninformative output ("undefined
undefined undefined").
This change removes the default argument, restoring a TypeError if there
is no argument. This also will restore our test coverage to 100%. (The
default argument is not tested in our current test suite.)
PR-URL: https://github.com/nodejs/node/pull/12843
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
- change URIError constructor to regular expression in assert.throws
- use block-scope for tests that spans multiple statements
PR-URL: https://github.com/nodejs/node/pull/12661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
PR-URL: https://github.com/nodejs/node/pull/12702
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/12703
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/12831
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/12594
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Use a regex to validate the error message.
PR-URL: https://github.com/nodejs/node/pull/12785
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
To avoid '[DEP0013] DeprecationWarning:
Calling an asynchronous function without callback is deprecated.'
PR-URL: https://github.com/nodejs/node/pull/12804
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
To avoid '[DEP0013] DeprecationWarning:
Calling an asynchronous function without callback is deprecated.'
PR-URL: https://github.com/nodejs/node/pull/12795
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/12792
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Using `assert.AssertionError()` without the `new` keyword results
in a non-intuitive error:
```js
> assert.AssertionError({})
TypeError: Cannot assign to read only property 'name' of function 'function ok(value, message) {
if (!value) fail(value, true, message, '==', assert.ok);
}'
at Function.AssertionError (assert.js:45:13)
at repl:1:8
at realRunInThisContextScript (vm.js:22:35)
at sigintHandlersWrap (vm.js:98:12)
at ContextifyScript.Script.runInThisContext (vm.js:24:12)
at REPLServer.defaultEval (repl.js:346:29)
at bound (domain.js:280:14)
at REPLServer.runBound [as eval] (domain.js:293:12)
at REPLServer.onLine (repl.js:545:10)
at emitOne (events.js:101:20)
>
```
The `assert.AssertionError()` can only be used correctly with `new`,
so this converts it into a proper ES6 class that will give an
appropriate error message.
This also associates the appropriate internal/errors code with all
`assert.AssertionError` instances and updates the appropriate test
cases.
PR-URL: https://github.com/nodejs/node/pull/12651
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Now that `node debug` is an alias for `node inspect`, it's possible that
`node-debug-pid` can run reliably. Modify for current behavior and move
from `disabled` to `parallel`.
PR-URL: https://github.com/nodejs/node/pull/12770
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
This is first in a hoped-for series of moves away from a monolithic
common.js that is loaded for every test and towards a more modular
approach. (In the end, common.js will hopefully contain checks for
variables leaking into the global space and perhaps some of the more
ubiquitous functions like common.mustCall().)
Move the WPT testing code to its own module.
PR-URL: https://github.com/nodejs/node/pull/12736
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>