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 E2E warnings #22621

Closed
wants to merge 3 commits into from
Closed

Fix E2E warnings #22621

wants to merge 3 commits into from

Conversation

CodingItWrong
Copy link
Contributor

Fixes two types of warnings that occur when running E2E tests:

  1. A deprecation warning from Jest: 'Option "setupTestFrameworkScriptFile" was replaced by configuration "setupFilesAfterEnv", which supports multiple paths.'
  2. YellowBox warnings when running the app in debug mode, about components that are deprecated and that require main queue setup.

By fixing these warnings, we increase contributors' confidence that things are working correctly, and draw attention to any warnings that they should pay attention to, if and when they arise.

I feel confident that we should hide the deprecated-component warnings; we want to use these components because we want them to be tested, until they're removed entirely.

For the warning "Module RCTImagePickerManager requires main queue setup", if that's something that can be fixed with reasonable effort in the RNTester code then I think it would be better to do so. Otherwise, I think it is good to hide the warning, because this is a condition we expect: it's not something a contributor should pay attention to.

Test Plan:

Run:

detox build -c ios.sim.debug
detox test -c ios.sim.debug

You should not see any warnings in the console, or any YellowBox warnings in the simulator.

Changelog:

[General] [Fixed] - Fix E2E warnings

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Tests This PR adds or edits a test case. labels Dec 12, 2018
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 ✅

Thanks for the PR Josh!

@CodingItWrong
Copy link
Contributor Author

Objc: ld: symbol(s) not found for architecture x86_64—seems like flake, quite unlikely to be related to this PR

@CodingItWrong
Copy link
Contributor Author

I reconfirmed the Detox tests work for me locally. Fixing whatever broke the Objc tests might fix Detox as well; I think we should not worry about the Detox ones yet.

@pull-bot
Copy link

Warnings
⚠️

🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Dec 14, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@CodingItWrong merged commit ac30f64 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Dec 14, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Dec 14, 2018
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Jul 18, 2019
Summary:
Fixes two types of warnings that occur when running E2E tests:

1. A deprecation warning from Jest: 'Option "setupTestFrameworkScriptFile" was replaced by configuration "setupFilesAfterEnv", which supports multiple paths.'
2. YellowBox warnings when running the app in debug mode, about components that are deprecated and that require main queue setup.

By fixing these warnings, we increase contributors' confidence that things are working correctly, and draw attention to any warnings that they _should_ pay attention to, if and when they arise.

I feel confident that we should hide the deprecated-component warnings; we _want_ to use these components because we want them to be tested, until they're removed entirely.

For the warning "Module RCTImagePickerManager requires main queue setup", if that's something that can be fixed with reasonable effort in the RNTester code then I think it would be better to do so. Otherwise, I think it is good to hide the warning, because this is a condition we expect: it's not something a contributor should pay attention to.
Pull Request resolved: facebook/react-native#22621

Differential Revision: D13468553

Pulled By: hramos

fbshipit-source-id: 1a5952087dd6fcc9ba08ff7a60ad9f5b075bef57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Tests This PR adds or edits a test case.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants