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

Alternate "slim" version of empower. #20

Closed
jamestalmage opened this issue Nov 24, 2015 · 6 comments
Closed

Alternate "slim" version of empower. #20

jamestalmage opened this issue Nov 24, 2015 · 6 comments

Comments

@jamestalmage
Copy link
Contributor

@twada

Poking through the code, I think there is room for a "slim" version of empower.

If my understanding is correct, then the following:

saveContextOnRethrow: true,
modifyMessageOnRethrow: false

Does not require a formatter at all. It just attaches the context and re-throws a copy.

In the "slim" version I am picturing, it would only take a patterns argument.

Also, instead of re-throwing an error (which messes with the existing stack trace needlessly). You could just use serialize-error attach the context, and throw.

We would format and print the message on the main thread using powerAssertContext.

Is what I am saying making sense?

@twada
Copy link
Member

twada commented Nov 24, 2015

@twada
Copy link
Member

twada commented Nov 24, 2015

Let me explain a history behind the code.

Also, instead of re-throwing an error (which messes with the existing stack trace needlessly). You could just use serialize-error attach the context, and throw.

I've implemented the same strategy, but some of old browsers (including PhantomJS) have strange behavior. When I rethrow Errors after attaching some props, the props disappear.

try {
    try {
        ...
    } catch (e) {
        e.foo = 'bar';
        throw e;
    }
} catch (e) {
    assert(e.foo === 'bar');  // Error on old PhantomJS
}

refactor(empower): always re-instantiate AssertionError on rethrow · power-assert-js/empower@997da24

However, I found that the attached props are remaining on current PhantomJS.
So I can just attach and rethrow the original Error now.

It's a delightful discovery. Thanks!

@jamestalmage
Copy link
Contributor Author

@twada - serialize-error actually does not create an error. It creates a plain old javascript object with deep copies of the Errors properties. An Error object does not survive JSON.stringify() very well. Especially assertion errors that have circular references.

Check the unit tests for all it does.

For use outside AVA, throwing non-errors is probably a bad idea. Could you maybe split most of the functionality into another module? empower-core or something. It should take a copyAssertion callback. empower can continue to copy the Assertion the way you've been doing it here, and we can make a custom version that uses serialize-error to make the copy.

@twada
Copy link
Member

twada commented Nov 25, 2015

@jamestalmage

Could you maybe split most of the functionality into another module? empower-core or something. It should take a copyAssertion callback.

Yeah I'm working on that !

@twada
Copy link
Member

twada commented Dec 4, 2015

Update: Now in a beta stage

@twada
Copy link
Member

twada commented Jan 6, 2016

Close this issue since twada/empower-core is going on.

@twada twada closed this as completed Jan 6, 2016
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

2 participants