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

Introduce jest snapshot testing utils (fixes #135) #297

Merged
merged 2 commits into from
Oct 21, 2019

Conversation

dogoku
Copy link
Contributor

@dogoku dogoku commented Dec 14, 2018

The main benefit of this PR is actually the applyTransform function, since it can be used for a lot of things.

  • Refactored runInlineTest and extracted applyTransform function
  • Introduced runSnapshotTest, same as runInlineTest but takes no expectedOutput
  • Introduced defineSnapshotTest, same as defineInlineTest but takes no expectedOutput
  • runInlineTest and runSnapshopTest now both also return the result applyTransform
    • This can be used to take the result of one run and use it for another test (check for idempotent)

- Refactored `runInlineTest` and extracted `applyTransform` function
- Introduced `runSnapshotTest`, same as `runInlineTest` but takes no `expectedOutput`
    - maybe worth merging the two, but that requires breaking change
- Introduced `defineSnapshotTest`, same as `defineInlineTest` but takes no `expectedOutput`
    - maybe worth merging the two, but that requires breaking change
- `runInlineTest` and `runSnapshopTest` now both also return the result `applyTransform`
    - This can be used to take the result of one run and use it for another (check for idempotent)
@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@fkling fkling requested a review from Daniel15 January 16, 2019 19:38
@ericwooley
Copy link

Any chance for a bump on this. It's been hanging for quite a while, and would make life so much easier.

@Daniel15
Copy link
Member

Daniel15 commented Sep 1, 2019

I can try to review this soon! In the meantime you might find the existing test utils useful.

@Hypnosphi
Copy link

I've published this fork as a temporary scoped package: https://www.npmjs.com/package/@hypnosphi/jscodeshift

Copy link
Member

@Daniel15 Daniel15 left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

@Daniel15 Daniel15 merged commit 8512b69 into facebook:master Oct 21, 2019
@dogoku dogoku deleted the patch-2 branch October 21, 2019 21:14
@Hypnosphi
Copy link

@jbrown215 does 0.7.0 include this? If so, can you please mention it in the changelog?

@Daniel15
Copy link
Member

Looks like it's included: v0.6.4...v0.7.0

jbrown215 added a commit to jbrown215/jscodeshift that referenced this pull request Dec 12, 2019
@jbrown215
Copy link
Contributor

@Hypnosphi: Yes and yes

@Daniel15
Copy link
Member

For what it's worth, I was just writing a transform today and really like that you separated out applyTransform method, because it means I can use it for inline snapshots (which I prefer over 'regular' snapshots). Thanks! :D

const {applyTransform} = require('jscodeshift/dist/testUtils');
const fooToBar = require('../fooToBar');

function transform(input, options = null) {
  return applyTransform(fooToBar, options, {source: input});
}

test('first', () => {
  expect(
    transform(`foo()`),
  ).toMatchInlineSnapshot(`"bar();"`);
});

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

Successfully merging this pull request may close these issues.

6 participants