-
Notifications
You must be signed in to change notification settings - Fork 477
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
Adding the ability to write tests inline, without fixture files #204
Conversation
} | ||
|
||
const output = transform( | ||
{path: inputPath, source}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one thing that feels a bit gross in this change is that inline tests aren't able to have a path here. It seems like most transforms only use source, but people will be unable to use inline tests with their transform if they use the file path directly.
cc @Daniel15 who built the test utils |
This seems reasonable to me... I'll take a closer look when I'm at my
computer.
I've also been meaning to change the test util to use Jest's snapshot
feature (keep the existing stuff for backwards compatibility, and use
snapshots when an output file isn't found so it's used for any future
tests).
Sent from my phone.
…On Jun 17, 2017 1:56 AM, "Felix Kling" ***@***.***> wrote:
cc @Daniel15 <https://github.com/daniel15> who built the test utils
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#204 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAFnHQqMmcruWZD65MIxtzuvC3j8S_Biks5sEqWegaJpZM4N769y>
.
|
Yeah, I was thinking about the snapshot test option as I was writing this. I think the snapshot test piece will be able to use the |
// Handle ES6 modules using default export for the transform | ||
const transform = module.default ? module.default : module; | ||
|
||
// Jest resets the module registry after each test, so we need to always get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like these comments have gone missing - Could you please re-insert them?
This looks pretty good to me! Please re-insert the comments that have been deleted:
And I'll merge this 😃 |
be22263
to
a7a5918
Compare
Whoops. Good catch. Updated. |
Thank you! :D |
Closes #203