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

power-assert feedback #96

Closed
sindresorhus opened this issue Oct 24, 2015 · 24 comments
Closed

power-assert feedback #96

sindresorhus opened this issue Oct 24, 2015 · 24 comments
Labels

Comments

@sindresorhus
Copy link
Member

Opening this just to gather some feedback about the new power-assert feature.

@sindresorhus
Copy link
Member Author

test(t => {
    const foo = 'foo';
    t.ok(foo.indexOf('bar') !== -1);
    t.end();
});
t.ok(foo.indexOf('bar') !== -1)
           |
           -1

@twada Any way we could show the value of foo here too? I think that could be useful.
@vdemedes What do you think?

@Qix-
Copy link
Contributor

Qix- commented Oct 24, 2015

This is badass. This might actually make me switch to Ava. 💃

@sindresorhus
Copy link
Member Author

@twada

Currently this:

test(t => {
    const b = 'bar';
    const c = 'baz';
    t.is(b, c);
    t.end();
});

Outputs:

t.is(b, c)
       |  | 
       |  "baz"
       "bar"

Is there any way to have it output the old simple output when using direct inputs? Meaning no expressions.

Old simple output:

'bar' === 'baz'

That output is faster to look at.

sindresorhus added a commit that referenced this issue Oct 24, 2015
@sindresorhus
Copy link
Member Author

@twada @uiureo I've documented the enhanced asserts. Would appriciate your review. Happy to change anything. 89219ce?short_path=0730bb7#diff-0730bb7c2e8f9ea2438b52e419dd86c9 :)

@vadimdemedes
Copy link
Contributor

This is fucking amazing.

@sindresorhus, agree on the simple output, looks way better!

@twada
Copy link
Contributor

twada commented Oct 24, 2015

First, as an author of power-assert, I'm so glad to see this happen! 😄
Thank you @sindresorhus !

@twada @uiureo I've documented the enhanced asserts. Would appriciate your review. Happy to change anything. 89219ce?short_path=0730bb7#diff-0730bb7c2e8f9ea2438b52e419dd86c9 :)

I think it's well written.

@twada
Copy link
Contributor

twada commented Oct 24, 2015

@sindresorhus

t.ok(foo.indexOf('bar') !== -1)
         |
         -1

@twada Any way we could show the value of foo here too? I think that could be useful.

AVA's power-assert output is configured at ava/enhance-assert.js (see renderers section)

If you change powerAssertRenderers.SuccinctRenderer to powerAssertRenderers.DiagramRenderer like below,

    empower(assert,
        powerAssertFormatter({
            renderers: [
                powerAssertRenderers.AssertionRenderer,
                powerAssertRenderers.DiagramRenderer
            ]
        }),
        {
            destructive: true,
            modifyMessageOnRethrow: true,
            saveContextOnRethrow: false,
            patterns: module.exports.PATTERNS
        }
    );

You'll get verbose output like

t.ok(foo.indexOf('bar') !== -1)
     |   |              |
     |   |              |
     |   -1             false
     ["toto", "tata", "titi"]

It seems nice to have verbosity configuration options in AVA, and change power-assert renderers according to the option value.

@sindresorhus
Copy link
Member Author

I know I'm asking a lot, but I would like the following not to use power-assert, since it's just direct input.

test(t => {
    const b = 'bar';
    const c = 'baz';
    t.is(b, c);
    t.end();
});

But this should use power-assert since it's an expression and could benefit from the power-assert output.

test(t => {
    const b = 'bar';
    const c = 'baz';
    const d = 'faz';
    t.is(b + d, c);
    t.end();
});

@twada
Copy link
Contributor

twada commented Oct 24, 2015

@sindresorhus

Is there any way to have it output the old simple output when using direct inputs? Meaning no expressions.

Currently, t.is is configured to be enhanced by power-assert and power-assert cannot toggle output on and off by captured value for now.

What you mean here is that if two arguments are both in form of Identifier or Literal, and its values are simple primitives, AVA's power-assert feature should be turned off automatically. It is not possible now but worth challenging.

@twada
Copy link
Contributor

twada commented Oct 24, 2015

@sindresorhus

Oh I missed your next question!

But this should use power-assert since it's an expression and could benefit from the power-assert output.

test(t => {
    const b = 'bar';
    const c = 'baz';
    const d = 'faz';
    t.is(b + d, c);
    t.end();
});

Thank you for your clear example!
My answer is the same as former one --- It's not possible now, but I'll try to make it happen.

@sindresorhus
Copy link
Member Author

It wouldn't nessecarily have to be turned off, could maybe just change the renderer to not show diagrams when Identifier or Literal is passed. This is just a nitpick/nicetohave, though.

@twada
Copy link
Contributor

twada commented Oct 24, 2015

@sindresorhus Thanks, it's getting a bit easier.

@vadimdemedes
Copy link
Contributor

@twada is babel-plugin-espower compatible with babel 6?

@twada
Copy link
Contributor

twada commented Oct 31, 2015

@vdemedes Not yet. I'm wrestling with Babel6 now.
Almost done but still doesn't work with presets.

@SamVerschueren
Copy link
Contributor

Not sure if I should create a new issue for this, but when using promises (await statements), power-assert does not show the correct value.

t.same(await fn(fixture1), {})
             |
             Object{}

The result of the promise is not empty, that's for sure :).


Seems I was too fast on this one. Apparently, the object shown underneath is the same as the second argument of the assert, which is not the actual result.

t.same(await fn(fixture1), {foo: 'bar'})
             |
             Object{foo: "bar"}

@twada
Copy link
Contributor

twada commented Nov 4, 2015

@SamVerschueren Thank you for reporting!
Currently, power-assert only supports ES6/ES2015 expressions. So AwaitExpression is not supported yet and to be supported in next version. BTW, I'll take a look about this behavior.

@SamVerschueren
Copy link
Contributor

Thanks for the feedback!

@sindresorhus
Copy link
Member Author

@SamVerschueren Looks like #113.

@SamVerschueren
Copy link
Contributor

Sorry, should have looked in the issue list first.

@twada
Copy link
Contributor

twada commented Nov 4, 2015

@SamVerschueren No problem!

@jamestalmage
Copy link
Contributor

@SamVerschueren - async/await fix is in master.

@SamVerschueren
Copy link
Contributor

Awesome! 👍

@novemberborn
Copy link
Member

@sindresorhus is there any more feedback you're looking to collect in this issue, or shall we close it?

@sindresorhus
Copy link
Member Author

Yup, can be closed in favor of twada/power-assert-renderers#3.

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

No branches or pull requests

7 participants