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

Tests failing on node 6.5.0 #86

Closed
blacksun1 opened this issue Aug 30, 2016 · 2 comments
Assignees
Labels
Milestone

Comments

@blacksun1
Copy link
Contributor

@blacksun1 blacksun1 commented Aug 30, 2016

Test "invalidates assertion (known type)" fails when run on Node 6.5.0 as per travis build https://travis-ci.org/hapijs/code

fails because nodes util.inspect has changed it's return value for the code const Custom = function () { };

"use strict";
const Custom = function () { };
const NodeUtil = require("util");
console.log(NodeUtil.inspect(new Custom()));

v4.4.7 returns {}
v6.4.0 returns {}
v6.5.0 returns Custom {}

My guess is that the other issues are similar but I haven't drilled down into them yet.

As proof, the following does work

it('invalidates assertion (known type)', (done) => {

    const Custom = function () { };
    Custom.prototype.inspect = function () {

        return '{}';
    };

    let exception = false;
    try {
        Code.expect(new Custom()).to.be.an.error(Error);
    }
    catch (err) {
        exception = err;
    }

    Hoek.assert(exception.message === 'Expected {} to be an error with Error type', exception);
    done();
});

or by assuming that Node can be fickle and just changing the expectation to be

const customInspection = NodeUtil.inspect(new Custom());
Hoek.assert(exception.message === `Expected ${customInspection} to be an error with Error type`, exception);

With the important difference being the custom inspect method on the Custom prototype. I don't mind helping out with fixing the errors but what do you want the output to be?

I think that Custom {} is better than {} in which case how about we just change the assertion to check for the string with the include node generated inspect string as above.

@cjihrig cjihrig closed this in #87 Sep 1, 2016
@cjihrig cjihrig added the test label Sep 1, 2016
@cjihrig cjihrig added this to the 4.0.0 milestone Sep 1, 2016
@cjihrig cjihrig self-assigned this Sep 1, 2016
@cjihrig

This comment has been minimized.

Copy link
Contributor

@cjihrig cjihrig commented Sep 1, 2016

Thanks for reporting this. It's now fixed.

@blacksun1

This comment has been minimized.

Copy link
Contributor Author

@blacksun1 blacksun1 commented Sep 2, 2016

No worries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.