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

Fix tests in template project (0.58-stable) #23128

Merged
merged 3 commits into from
Jan 23, 2019

Conversation

vovkasm
Copy link
Contributor

@vovkasm vovkasm commented Jan 23, 2019

This fixes template application sample tests.

  1. Fix adding tests folder to npm package
  2. Fix tests itself (add babel-core to deps and rename .babelrc to babel.config.js)

Unfortunately some changes can't be applied to master branch because of cli extraction. Moreover some changes (probably addition of babel-core to deps) will not be needed when jest 24 will be released. So this is path specially for 0.58-stable branch.

I think after apply this, we also should do some things:

  1. Recheck npmignore patterns in master branch, remove unused and ensure that templates packed correctly.
  2. Recheck our deps and config in HelloWorld template after jest 24 will be released.

Changelog:

[General] [Fixed] - Fix tests in template HelloWorld application

Test Plan:

# Prepare RN tarball for testing
cd /Volumes/work/react-native
npm pack
# clear yarn cache
yarn cache clean

# Test with yarn
cd /Volumes/work/tmp
react-native init --version /Volumes/work/react-native/react-native-0.58.0-rc.3.tgz Sample1
cd Sample1
yarn test
# Tests passed

# Test without yarn (yarn not available on system)
cd /Volumes/work/tmp
react-native init --version /Volumes/work/react-native/react-native-0.58.0-rc.3.tgz Sample1
cd Sample1
npm test
# Tests passed

# Recheck dependencies with fresh reinstall
cd /Volumes/work/tmp/Sample1
rm -rf node_modules yarn.lock package-lock.json
npm install
npm test
# Tests passed

PR facebook#18019 Removed __tests__ and __fixtures__ folders from npm package.
But it also effectively removed __tests__ folder from initial project
template. This commit restores __tests__ in template application.

It applicable only to 0.58-stable branch because in master, templates
was moved to another location. I think we should recheck what is ignored
in master after this landed.
… init new project. This should fix tests with babel7.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 23, 2019
@pull-bot
Copy link

Warnings
⚠️

📋 Release Notes - This PR appears to be missing Release Notes.

⚠️

❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

Generated by 🚫 dangerJS

Copy link
Contributor

@kelset kelset left a comment

Choose a reason for hiding this comment

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

LGTM

@hramos
Copy link
Contributor

hramos commented Jan 23, 2019

Thanks for working on this! Since the PR is against 0.58-stable, I can't land it, but @grabbou or anyone else with write access who is working on the 0.58 release can push these changes to the 0.58-stable branch directly. Let me know if I can help with that.

@grabbou grabbou merged commit 23cfa29 into facebook:0.58-stable Jan 23, 2019
@grabbou
Copy link
Contributor

grabbou commented Jan 23, 2019

Awesome! Thank you so much for working on it! Shall we also make sure this works on master? I think the only difference is that there's no templates/HelloWorld but template.

@vovkasm
Copy link
Contributor Author

vovkasm commented Jan 23, 2019

@grabbou Yes, I'll check and make PRs needed for land this changes to master. I think that even changes to react-native-cli will not be needed, because:

  1. setup babel-core@7.0.0-bridge.0 as dependency should not be needed because master currently at jest@24
  2. modification of generator (https://github.com/facebook/react-native/pull/23128/files#diff-e1bddafdbb592817072925c6b7aabcd2R130) should not be needed because babel.config.js probably can live in template with its original name. Because by definition babel.config.js only processed at project root.

t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
* Restore __tests__ folder in HelloWorld template

PR facebook#18019 Removed __tests__ and __fixtures__ folders from npm package.
But it also effectively removed __tests__ folder from initial project
template. This commit restores __tests__ in template application.

It applicable only to 0.58-stable branch because in master, templates
was moved to another location. I think we should recheck what is ignored
in master after this landed.

* Use babel.config.js in template HelloWorld application

* Add babel-core@^7.0.0-bridge.0 to jest dependencies when react-native init new project. This should fix tests with babel7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants