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

update to new power-assert-runtime #903

Merged
merged 3 commits into from
Jun 5, 2016
Merged

Conversation

nfcampos
Copy link
Contributor

@nfcampos nfcampos commented Jun 4, 2016

maybe @twada can check whether I correctly configured the various power-assert things

@twada
Copy link
Contributor

twada commented Jun 4, 2016

Thanks @nfcampos! This is exactly what I wanted to do today 😃

babel-plugin-espower 2.2.0 and the new power-assert-runtime comes with many architectural improvements with less dependencies. I hope AVA guys will like it.

@jamestalmage
Copy link
Contributor

jamestalmage commented Jun 4, 2016

I'm sure we will!

@twada, has the runtime dependency graph been reduced? Should we move rendering back into the child processes - we've had lots of problems trying to serialize large objects for rendering in the main thread.

@twada
Copy link
Contributor

twada commented Jun 5, 2016

has the runtime dependency graph been reduced?

Yeah, reducing dependencies is one of the goals of new power-assert-runtime. There's no extra parser anymore. Many ponyfills are consolidated into single core-js.

I also recommend @nfcampos to update empower-core to 0.6.1 to reduce dependencies much more.

Should we move rendering back into the child processes

Yes. Now we can render power-assert output in subprocesses.

@jamestalmage
Copy link
Contributor

@nfcampos. Can you bump the dependency as mentioned? Otherwise this looks good to me.

We can tackle moving rendering to the child process in another PR

@nfcampos
Copy link
Contributor Author

nfcampos commented Jun 5, 2016

done

@twada
Copy link
Contributor

twada commented Jun 5, 2016

Thanks @nfcampos, looks good to me! 👍

@jamestalmage jamestalmage merged commit fb3af50 into avajs:master Jun 5, 2016
@jamestalmage
Copy link
Contributor

jamestalmage commented Jun 5, 2016

Thanks @nfcampos! Want to work on moving the renderer into the child process?

The idea is to go back to the way power-assert behaves in other contexts - replacing the message on the error.

@nfcampos
Copy link
Contributor Author

nfcampos commented Jun 5, 2016

@jamestalmage yes, I'll do that, if I understand correctly it's something like using https://github.com/power-assert-js/empower instead of empower-core in enhance-assert.js and stop using the formatter in run-status.js?

@nfcampos nfcampos deleted the power branch June 5, 2016 09:57
@twada
Copy link
Contributor

twada commented Jun 5, 2016

@nfcampos One thing to notice is that empower module is now a thin wrapper around empower-core for power-assert internal compatibility and there is no reason to use empower in AVA. It's better to keep using empower-core directly.

@nfcampos
Copy link
Contributor Author

@twada yeah you're right

@jimthedev
Copy link

@nfcampos So, quick question about the ava docs and this PR, specifically in the React recipe for JSX helpers there the following note:

Note that you have to use variables like actual and expected because power-assert doesn't handle JSX correctly.

The text links to this issue which although open (documentation still pending) states that it will be available in ava as soon as this PR (#903) is merged. Since #903 has been merged, do we need to remove that note from the React recipe?

@nfcampos
Copy link
Contributor Author

yes that note should be removed, that should now work

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

Successfully merging this pull request may close these issues.

Feature Request: Better error messaging for acorn parsing errors when testing React
4 participants