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

Coverage && transform refactor #1321

Merged
merged 3 commits into from
Jul 29, 2016
Merged

Coverage && transform refactor #1321

merged 3 commits into from
Jul 29, 2016

Conversation

aaronabramov
Copy link
Contributor

No description provided.

@aaronabramov
Copy link
Contributor Author

this uses babel-plugin-istanbul for instrumenting the code. Right now it will run the instrumentation step on top of whatever is already transformed.

next step will be combining the instrumentation with babel-jest or custom preprocessor (www)

@ghost ghost added the CLA Signed ✔️ label Jul 26, 2016
@aaronabramov aaronabramov force-pushed the coverage branch 2 times, most recently from ffe37ee to 43b2d67 Compare July 26, 2016 23:51
@ghost ghost added the CLA Signed ✔️ label Jul 26, 2016
wrappedResult = wrap(content);
}

return new vm.Script(wrappedResult, {displayErrors: true, filename});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: better error handling around this call. All SyntaxError in the transformed source code will throw here

.*/packages/jest-react-native/node_modules/react-native/.*
.*/node_modules/fbjs/.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you do the same for react-native, since you are at it?

`\nScript: ${content}\n` +
`Path: ${filename}\n` +
`Config: ${stableStringify(config, {space: 2})}`
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep this? I actually wanted to have the config and the original script in the snapshot.

@cpojer
Copy link
Member

cpojer commented Jul 28, 2016

This is great work. I left a bunch of minor comments but overall I love the direction this is taking and appreciate all the cleanups you are making, especially in jest-runtime.

I think one thing that's missing here is that test files themselves don't get ignored from coverage instrumentation any more. Also, please remove this._testRegex from the runtime. I believe it is not needed any more.

Can you add a test for the transform test that spits out instrumented code into a snapshot?

Feel free to merge after fixing the minor nits. Great work!

@aaronabramov
Copy link
Contributor Author

updated

@ghost ghost added the CLA Signed ✔️ label Jul 29, 2016
@cpojer cpojer merged commit 0ec5ec1 into jestjs:master Jul 29, 2016
@aaronabramov aaronabramov deleted the coverage branch June 8, 2017 18:01
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants