-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
☂️✅ Green CI: Re-enabling disabled tests #23561
Comments
@hramos is there more detail on what this should do? |
I haven’t looked into it recently. There’s a PR out right now that reworks how CocoaPods is supported in this repo, so it’s an open question whether this test is still necessary or sufficient. Edit: oh, it’s your PR :) Feel free to remove / provide an updated replacement for testing our CocoaPods support as part of your PR. |
Summary: Do not run disabled tests, even when the commit / PR is pushed by hramos. See the existing functionality at https://circleci.com/gh/facebook/react-native/73844?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link: ![screen shot 2019-02-20 at 8 21 39 am](https://user-images.githubusercontent.com/165856/53106776-90ecb600-34e8-11e9-9b02-a7f3990407b2.png) There are a handful of tests that are known to be broken in Circle CI. This has been the case since at least Fall 2017, when we migrated to Circle 2.0. These tests haven't been fixed for several reasons, one of them being that once they were removed from the Circle CI config, the pressure to fix them has been lowered. Last year, I added these disabled tests back to Circle CI, but used a script to prevent them from running unless the job was initiated by myself. This would allow us to get good signal from Circle CI without polluting results with known failing tests, while still showing me which tests needed some work before getting re-enabled again. In practice, this functionality is introducing more friction as I work on fixing new CI failures: my own fixup PRs are marked as failing due to these "disabled tests" (see test_android on #23558). To ensure we don't lose track of these failures, I've created an umbrella issue at #23561. Pull Request resolved: #23562 Differential Revision: D14161172 Pulled By: cpojer fbshipit-source-id: 040500dcb433d3127c64a42b31f94af6bbaa6ed1
@hramos I'd like to help with this |
Summary: Do not run disabled tests, even when the commit / PR is pushed by hramos. See the existing functionality at https://circleci.com/gh/facebook/react-native/73844?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link: ![screen shot 2019-02-20 at 8 21 39 am](https://user-images.githubusercontent.com/165856/53106776-90ecb600-34e8-11e9-9b02-a7f3990407b2.png) There are a handful of tests that are known to be broken in Circle CI. This has been the case since at least Fall 2017, when we migrated to Circle 2.0. These tests haven't been fixed for several reasons, one of them being that once they were removed from the Circle CI config, the pressure to fix them has been lowered. Last year, I added these disabled tests back to Circle CI, but used a script to prevent them from running unless the job was initiated by myself. This would allow us to get good signal from Circle CI without polluting results with known failing tests, while still showing me which tests needed some work before getting re-enabled again. In practice, this functionality is introducing more friction as I work on fixing new CI failures: my own fixup PRs are marked as failing due to these "disabled tests" (see test_android on facebook#23558). To ensure we don't lose track of these failures, I've created an umbrella issue at facebook#23561. Pull Request resolved: facebook#23562 Differential Revision: D14161172 Pulled By: cpojer fbshipit-source-id: 040500dcb433d3127c64a42b31f94af6bbaa6ed1
Summary: As part of facebook#23561 this is an attempt at fixing iOS. [iOS] [Fixed] - e2e test Pull Request resolved: facebook#23566 Differential Revision: D14162780 Pulled By: cpojer fbshipit-source-id: b55d32e30f88370100f7fbddf9dfb208280844f4
@gregoryfm it looks like @ericlewis is already taking a stab at this, through #23571. |
Any help with android e2e would be much appreciated tho! |
Summary: Part of #23561. Small refactor to use 1 marker for the different tests, as it doesn't matter where it is- so long as we can detect it. [General] [Fixed] - turn on JS e2e tests Pull Request resolved: #23571 Differential Revision: D14172069 Pulled By: hramos fbshipit-source-id: cdde369a09d3528d05fec01d015613b3397714e6
@hramos I think the iOS portion is complete too, it’s e2e test consists of setting up a project via |
The js e2e tests consist of us being able to bundle the js successfully, and has been completed as well, as marked. But wanted to note how it works for transparency purposes. |
Working on fixing the android e2e tests! Once I've made some (more) progress, I'll give an update :) |
Summary: This PRs makes an attempt at fixing the set up of the Android end to end tests, and the tests themselves. The end goal is to re-enable the tests on CircleCI (see #23561 for more details). The goal of this PR is to the end to end tests to a working state. Better tests can be added at a later point. I changed the tests using the menu button. These tests made something silently crash/hang, after which it was no longer possible to get an element or even get the source. A fix for this needs further investigation. Also, I enabled the tests in the CircleCI config, however CircleCI is currently failing on them with the following error: ``` info Running /opt/android/platform-tools/adb -s emulator-5554 reverse tcp:8081 tcp:8081 error: closed info Could not run adb reverse: Command failed: /opt/android/platform-tools/adb -s emulator-5554 reverse tcp:8081 tcp:8081 info Starting the app on emulator-5554 (/opt/android/platform-tools/adb -s emulator-5554 shell am start -n com.endtoendtest/com.endtoendtest.MainActivity)... Starting: Intent { cmp=com.endtoendtest/.MainActivity } Too long with no output (exceeded 10m0s) ``` Some help here would be appreciated. An alternative is to not enable the tests yet on CircleCI in this PR. [Android] [fixed] - Fix and update end to end tests for Android Pull Request resolved: #23958 Differential Revision: D14502884 Pulled By: hramos fbshipit-source-id: 4316c3fd817451d332e64a10d88389b74a60d3dd
@casperboone @ericlewis thanks so much for your help on this. @hramos do you think you could give an update about what we still would like to do? I think there is probably a bunch more integration work for Facebook to keep things green but I'm not entirely up-to-date on what that work may be. |
Summary: Fixes the e2e detox build step by manually overriding `PROJECT_ROOT` for the project's JS bundle build step. After seeing [quite](#18472 (comment)) [a](#18472 (comment)) [few](#15432 (comment)) [comments](https://stackoverflow.com/a/49506072) suggesting running some variant of the `react-native bundle` manually on your own so as to build the jsbundle required as part of the build step for RNTester project... The main issue I found was that the working directory in which `react-native-xcode.sh` executed the CLI bundle step was not correct. The `PROJECT_ROOT` was not resolving to the root of the `react-native` project directory, but instead to something to the effect of `/Users/gibson.cheng/IdeaProjects/react-native/../..` - of which of course the build step would not be able to find the `react-native` project to run the build against. I'm not sure if new generated `react-native` projects require this manual override, so I only applied it to the RNTester project. Reviewers are welcome to correct my understanding and solutioning to this matter :) hramos, if this works, perhaps there would not be a need to push through with #24936. Also, this contributes to #23561. ## Changelog [Internal] [Fixed] - Fix build-ios-e2e build step Pull Request resolved: #24953 Differential Revision: D15415850 Pulled By: hramos fbshipit-source-id: baaff09f81f01be4da1608e0b2898d037db35c23
Thanks for all your help getting these tests fixed. I'm posting an update to clarify what needs to be done before this issue can be closed. The podspec tests and Android end-to-end tests remain disabled, and we could use some help getting them green again. iOS: Podspec testsThese are failing because the yoga pod cannot be built (expand to see logs):
While building module 'yoga' imported from /var/folders/82/w8hcc_tj7yq80482srkc1r2x0bj21j/T/CocoaPods-Lint-20190530-11880-m4ez3b-yoga/App/main.m:3:
In file included from :1:
In file included from /Users/hramos/Library/Developer/Xcode/DerivedData/App-fcnbolkgqgfrrdcvttlgzjkxdyvg/Build/Products/Release-iphonesimulator/yoga/yoga.framework/Headers/yoga-umbrella.h:13:
/Users/hramos/Library/Developer/Xcode/DerivedData/App-fcnbolkgqgfrrdcvttlgzjkxdyvg/Build/Products/Release-iphonesimulator/yoga/yoga.framework/Headers/CompactValue.h:11:10: fatal error: 'cmath' file not found
#include
^~~~~~~
1 error generated.
/var/folders/82/w8hcc_tj7yq80482srkc1r2x0bj21j/T/CocoaPods-Lint-20190530-11880-m4ez3b-yoga/App/main.m:3:9: fatal error: could not build module 'yoga'
@import yoga;
~~~~~~~^~~~
2 errors generated.
Testing with [!] yoga did not pass validation, due to 1 error. /Library/Ruby/Gems/2.3.0/gems/cocoapods-1.6.1/lib/cocoapods/command/lib/lint.rb:92:in The failure can be reproduced by running |
Summary: Fixes the e2e detox build step by manually overriding `PROJECT_ROOT` for the project's JS bundle build step. After seeing [quite](facebook#18472 (comment)) [a](facebook#18472 (comment)) [few](facebook#15432 (comment)) [comments](https://stackoverflow.com/a/49506072) suggesting running some variant of the `react-native bundle` manually on your own so as to build the jsbundle required as part of the build step for RNTester project... The main issue I found was that the working directory in which `react-native-xcode.sh` executed the CLI bundle step was not correct. The `PROJECT_ROOT` was not resolving to the root of the `react-native` project directory, but instead to something to the effect of `/Users/gibson.cheng/IdeaProjects/react-native/../..` - of which of course the build step would not be able to find the `react-native` project to run the build against. I'm not sure if new generated `react-native` projects require this manual override, so I only applied it to the RNTester project. Reviewers are welcome to correct my understanding and solutioning to this matter :) hramos, if this works, perhaps there would not be a need to push through with facebook#24936. Also, this contributes to facebook#23561. ## Changelog [Internal] [Fixed] - Fix build-ios-e2e build step Pull Request resolved: facebook#24953 Differential Revision: D15415850 Pulled By: hramos fbshipit-source-id: baaff09f81f01be4da1608e0b2898d037db35c23 (cherry picked from commit adc0878)
Summary: Fixes the e2e detox build step by manually overriding `PROJECT_ROOT` for the project's JS bundle build step. After seeing [quite](facebook#18472 (comment)) [a](facebook#18472 (comment)) [few](facebook#15432 (comment)) [comments](https://stackoverflow.com/a/49506072) suggesting running some variant of the `react-native bundle` manually on your own so as to build the jsbundle required as part of the build step for RNTester project... The main issue I found was that the working directory in which `react-native-xcode.sh` executed the CLI bundle step was not correct. The `PROJECT_ROOT` was not resolving to the root of the `react-native` project directory, but instead to something to the effect of `/Users/gibson.cheng/IdeaProjects/react-native/../..` - of which of course the build step would not be able to find the `react-native` project to run the build against. I'm not sure if new generated `react-native` projects require this manual override, so I only applied it to the RNTester project. Reviewers are welcome to correct my understanding and solutioning to this matter :) hramos, if this works, perhaps there would not be a need to push through with facebook#24936. Also, this contributes to facebook#23561. ## Changelog [Internal] [Fixed] - Fix build-ios-e2e build step Pull Request resolved: facebook#24953 Differential Revision: D15415850 Pulled By: hramos fbshipit-source-id: baaff09f81f01be4da1608e0b2898d037db35c23
I've updated the root issue with the latest list of disabled tests. I have opened #28392 to make it easier to re-enable tests and iterate on them on your PRs. |
JavaScript end-to-end testsThese run as part of the workflows:
version: 2
tests:
jobs:
...
- test_js:
run_disabled_tests: true
requires:
- setup_js Why it's failing
The e2e script creates a react-native package and initializes a new app based on the contents of the |
iOS end-to-end testsThese run as part of the workflows:
version: 2
tests:
jobs:
...
- test_ios:
run_disabled_tests: true
requires:
- setup_ios Why it's failing
|
CocoaPods Podspecs testsThese run as part of the To run them, edit workflows:
version: 2
tests:
jobs:
...
- test_ios:
run_disabled_tests: true
requires:
- setup_ios Why it's failingThe script refers to a subspec that no longer exists,
This was removed in 2321b3f#diff-66230b3e029caa37b0fbdc8cbd47f4ab. Re-enabling this test will require adding back the |
Android end-to-end testsThese run as part of the workflows:
version: 2
tests:
jobs:
...
- test_android:
run_disabled_tests: true
requires:
- setup_android Why it's failing
|
#28392 fixes the JavaScript e2e tests. |
Great stuff @hramos, I’ll dive into these soon too 👍 |
After upgrading a project with version 0.61.2 to 0.62.0 building the app fails with the same error. |
That's right, @timkuilman, these tests are broken on master. |
I'm sorry I wasn't very clear. After doing the upgrade as explained in the React Native Upgrade tool my app was no longer building. I'm not referring to the test. In my case, the error was fixed by enforcing the new build system of XCode. I had these lines from
|
Hi, I have the same issue here: "ld: library not found for -lCocoaAsyncSocket", " 0.62.2". |
For me it only worked when I added:
|
@hramos is it still worth keeping this issue open? Is it still relevant? |
I'm not sure why GH gave me a notification for this issue given that there's no activity (?) |
We have a handful of tests that have been disabled on Circle CI over the years. It's time to get these back up and running so we can merge PRs and cut new releases with confidence.
This is an umbrella task intended to track all the work necessary to re-enable these tests. This issue is being kept up to date so new items will be added and completed ones will be (re)moved. Some of the items will spawn separate issues to track work. If you don't see a name assigned to any of the below items and you would like to help out, please volunteer in the comments and reach out to me to get started!
What needs to be re-enabled?
These tests are configured as aliases in the Circle CI config. I'm happy to help with the Circle CI side of things, but for now you can focus on making sure the command/script (e.g.
./scripts/run-ci-e2e-tests.js --ios
) succeeds locally.The text was updated successfully, but these errors were encountered: