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

Switch mocha to jest #768

Merged
merged 1 commit into from
Feb 23, 2017
Merged

Conversation

akozhemiakin
Copy link
Contributor

@akozhemiakin akozhemiakin commented Feb 21, 2017

This commit replaces mocha with jest. Also this commit removes dependency on
babel-plugin-webpack-loaders plugin because of the following reasons:

  1. It seems that the author deprecated and abandoned this plugin and he suggests
    not to use it and to use mocks and native test utilities tools instead.
  2. This plugin conflicts with webpack 2 and because of the reason one it is unlikely
    that it will ever support it.

In order to remove dependency on the aforementioned plugin and keep using css
modules in the same time the following trick is used: All components that need to
be selected in the end-to-end test receive data-tid (TestID) and data-tclass
(TestCLASS) properties. It is necessary because without running webpack we can't
use class names to select components because we do not know what class names
css loader actually generates for us.

P.S. I prepared upgrade to webpack 2 but it depends on this commit. So after this PR is closed I'll create pull request for webpack upgrade.

@akozhemiakin akozhemiakin changed the title [WIP] Switch mocha to jest Switch mocha to jest Feb 21, 2017
@jhen0409
Copy link
Member

Could you also change test/example.js? Has lint errors in AppVeyor build, I guess Travis CI passed because node_modules cache.

@akozhemiakin
Copy link
Contributor Author

I fixed example.js but it seems that appveyor has some other reason to fail. I got very similar error one time and fixed it by deleting node_modules directory and reinstalling project. Do not know however how to fix appveyor.

@jhen0409
Copy link
Member

Actually @amilajack was do #613, we have the same problem on AppVeyor. :\

@akozhemiakin
Copy link
Contributor Author

akozhemiakin commented Feb 22, 2017

Maybe it expects \ instead of / ? Apveyor runs on windows after all. I'll check it.

@akozhemiakin
Copy link
Contributor Author

Hooray! I fixed it )

Copy link
Member

@amilajack amilajack left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just need to remove the mocha dependencies (mocha, eslint-plugin-mocha, etc). Also remove the mocha related eslint rules and plugin from .eslintrc and run npm run flow-typed to update type definitions to reflect jest

This commit replaces mocha with jest. Also this commit removes
dependency on babel-plugin-webpack-loaders plugin because of
the following reasons:
1. It seems that the author deprecated and abandoned this plugin
and he suggests not to use it and to use mocks and native test utilities
tools instead.
2. This plugin conflicts with webpack 2 and because of the reason one
it is unlikely that it will ever support it.

In order to remove dependency on the aforementioned plugin and
keep using css modules in the same time the following trick is used:
All components that need to be selected in the end-to-end test receive
data-tid (TestID) and data-tclass (TestCLASS) properties. It is necessary
because without running webpack we cannot use class names to select
components because we do not know what class names css loader
actually generates for us.
@akozhemiakin
Copy link
Contributor Author

Done. I believe I remove all references to mocha.

@amilajack
Copy link
Member

@akozhemiakin LGTM!

@amilajack amilajack merged commit a76aef2 into electron-react-boilerplate:master Feb 23, 2017
@akozhemiakin akozhemiakin deleted the jest branch February 23, 2017 18:04
@akozhemiakin
Copy link
Contributor Author

akozhemiakin commented Feb 23, 2017

Nice. Now @amilajack you can fill another checkbox here #504 (:

@amilajack
Copy link
Member

Done!

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