-
Notifications
You must be signed in to change notification settings - Fork 24.9k
Improve e2e times and flakiness by using the search #51590
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
Improve e2e times and flakiness by using the search #51590
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mateoguzmana, that's amazing, thank you so much!
We currently have a small issue with tests running on APIs: we also run the E2E tests in debug mode, to catch issues with metro, and debug is currently showing some warnings that are preventing the navigation to that tab. We already have a person looking into them to fix those warnings.
To shed some lights on the E2E tests: we use them for Releases. They are non supposed to be exhaustive, but more of a smoke test to inform the release crew if something really important stopped working. For this reason, we want them to run fast - so this PR is amazing! - not to slow down the release.
We should find the right balance between testing RNTester and release speed.
If the test number starts to slow down the release, we might have to split them in different flows and run them in parallel.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Thanks for all the info @cipolleschi!
I noticed this locally as well, I was going to suggest to trigger a command to attempt hide the warnings only if they are visible for tests going to that tab, but great to know you are looking into it.
Makes a lot of sense. Would it still be ok for me to send a few PRs for some test cases? They are non-exhaustive and I think it could they could help for smoke testing. For some of the Kotlin migrations I do quite a lot of checks, and wanted to automate that further so it's faster to test locally and with a bit more structure. (maybe I should join the testing release crew 😄) |
It depends how much time they add to the jobs! 😬 |
@cipolleschi merged this pull request in df6eff4. |
This pull request was successfully merged by @mateoguzmana in df6eff4 When will my fix make it into a release? | How to file a pick request? |
Summary: This is similar to #51590, but way better as it improves indirectly the flakiness for tests in the API tab. When the logbox is shown in debug mode, it interferes and sometimes makes that test fail, so this prevents that. Android also takes more advantage of the improvement with this change, the previous PR only improved significantly iOS. All the screens inside the RNTester seem to have a deeplink, which makes it easier to open the tests as the test cases are intended to check mostly specific behaviour of RN, and it is not necessary to have a middle step to find the specific components. Maybe it would be good to run this a few times in CI to see if there are no side effects or flakiness added by opening deep links on CI builds. ## Changelog: [INTERNAL] - Improve e2e times by using deep links to open examples Pull Request resolved: #51786 Test Plan: ```sh yarn e2e-test-android yarn e2e-test-ios ``` iOS: | Before | After | |--------|-------| | <img width="387" alt="image" src="https://github.com/user-attachments/assets/03ccd957-d401-4944-bb5c-d3e7db957b2e" /> | <img width="364" alt="image" src="https://github.com/user-attachments/assets/40a14c95-63f8-441d-b718-b5f57a506393" /> | Android: | Before | After | |--------|-------| | <img width="455" alt="image" src="https://github.com/user-attachments/assets/c71da8d0-df69-44af-b1b2-580995ce55c7" /> | <img width="449" alt="image" src="https://github.com/user-attachments/assets/7357e670-3510-4bbe-8543-68d3bd8c4bea" /> | Reviewed By: cipolleschi Differential Revision: D75938844 Pulled By: cortinico fbshipit-source-id: c7d4063af561e7b0e583eddefcbb289786f3805a
Summary:
I'm writing some tests for some APIs/components, and I see some opportunity for improvement in the way how we can write e2e cases for the RNTester.
This diff improves two things:
scrollUntilVisible
functionality, it takes quite some time for some cases, as some of the items are not very up in the lists and getting to them it's not always very fast.scrollUntilVisible
did not find anything as the scroll was too fast so the test ended up failing.Instead of using
scrollUntilVisible
for all cases, we can simply use the search bar which we have in both Components and APIs tabs. This runs faster and we can also share the search flow across multiple test cases, so writing the tests becomes a bit simpler as well.Initially, I did this for all cases but not all cases benefit from this change as some of them are easier to find than others – the improvement was most notable in iOS (keyboard visibility seems to make a big difference), but I still think it is a good baseline to use the search as if more test cases are added, likely, many of them are not going to be so easy to find.
Changelog:
[INTERNAL] - Improve e2e times and flakiness by using the search
Test Plan:
These are the differences in the time it takes to pass the tests locally.
iOS:
Android (no significant difference):