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

Replace gulp/mocha/mockery/istanbul with Jest #187

Merged
merged 7 commits into from
Mar 28, 2017
Merged

Replace gulp/mocha/mockery/istanbul with Jest #187

merged 7 commits into from
Mar 28, 2017

Conversation

mischah
Copy link
Member

@mischah mischah commented Mar 28, 2017

… and add a yarn lock file.

@SBoudrias I need your help 😣

Can’t get those damn tests green.
Reason: I really don’t understand the mocking part.
I checked how you update the tests in generator-node and read the Jest docs but can’t get my head around.

Could you please have a look?

… and add a yarn lock file.
@sindresorhus
Copy link
Member

Why Jest? I would be happy to do a PR for AVA. It would let us use async/await which simplifies the tests. Example: https://github.com/sindresorhus/generator-nm/blob/master/test.js

@mischah
Copy link
Member Author

mischah commented Mar 28, 2017

Hej Sindre,

Simon said:

I'd still be happy to merge a PR offering the option to generate AVA tests in the future.

See yeoman/generator-node#222 and yeoman/generator-node#251 for additional context.

Cheers, Michael

@sindresorhus
Copy link
Member

It's not really much context though. All I see is something like "We have two good candidates" and then it was done. And how is that even relevant to this generator?

@SBoudrias
Copy link
Member

@sindresorhus I chose Jest mainly because it's the one I used a fair amount - because it became the de-facto react test runner.

Also, the api is almost a 1-1 with mocha, so I felt it was a fairly easy upgrade for current users of generator-node/generator-generator. And there's 0 config to get code coverage; which was the main motivation to move away from Mocha.

Not saying it's necessarily better than ava, but it worked well for my use-cases and I knew the tool.

__tests__/app.js Outdated

mockery.registerMock('superb', () => {
beforeEach(() => {
jest.mock('superb', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think you want jest.mock('superb', () => () => 'cat\'s meow');

@SBoudrias
Copy link
Member

@mischah I think we should also add a .eslintignore file to have eslint ignore the templates. Rel to #131

@mischah
Copy link
Member Author

mischah commented Mar 28, 2017

@sindresorhus

And how is that even relevant to this generator?

Having the same test setup makes it easy to update generator-generator with running yo generator in the working dir of generator-generator when changes from generator-node needs to be applied.

See #186 (comment)

@mischah mischah changed the title WIP: Replace gulp/mocha/mockery/istanbul with Jest Replace gulp/mocha/mockery/istanbul with Jest Mar 28, 2017
@mischah
Copy link
Member Author

mischah commented Mar 28, 2017

@SBoudrias Thanks. Are we ready to squash and merge to get generator-generator 3.0.0 out on the streets?

@SBoudrias
Copy link
Member

Let's get the eslintignore before releasing v3

@mischah mischah merged commit f6cc9ba into master Mar 28, 2017
@mischah mischah deleted the v3 branch March 28, 2017 19:28
@SBoudrias
Copy link
Member

Hey @mischah I meant the .eslintignore should also be generated for generator-generator users, as they'll run into the same issue.

@mischah
Copy link
Member Author

mischah commented Mar 29, 2017

Oops. Makes sense. But shouldn't this be provided by generator-node? @SBoudrias

@SBoudrias
Copy link
Member

No because the template directory is only related to yeoman-generators

@mischah
Copy link
Member Author

mischah commented Mar 29, 2017

Sorry for the confusion. I’m going to tackle this tonight.

mischah added a commit that referenced this pull request Apr 5, 2017
No need to use this since we have a .eslintignore. Seems to sllipped trough #187
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.

3 participants