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

test(e2e): forward port to Detox test framework v18 #5734

Merged
merged 6 commits into from
Oct 1, 2021

Conversation

mikehardy
Copy link
Collaborator

@mikehardy mikehardy commented Sep 22, 2021

Description

This is in prep for support of iOS15 in Detox - currently Detox breaks on iOS15 but support (when it comes) will certainly rely on a new version, so it's finally time to forward port here

1- two tests needed to be disabled
-- one test exposed a Detox (I think) crash in their network sync code in analytics
-- one test was simply a failure in crashlytics restart detection - very odd

Both are marked with TODO (or FIXME) and detox

2- may be a little flaky on iOS, I noticed some crashes on iOS but they were not reproducible, only time + experience will tell - I have noted that here #4058 (comment) and logged it here wix/Detox#3000

3- because the app will never launch as part of the detox init now, the jet hot-patch to init that waited for node to be connected to the app needed to be removed. launchApp is now called separately and it was already hot-patched by jet to wait for connection so it is sufficient

4- sadly I continue to suffer from flakiness in the firestore pre-compiled framework inclusion so I had to comment it out again

5- because iOS 15 is not supported by Detox yet (wix/Detox#2895) I have opted to pin our iOS e2e workflow on Xcode 12.5.1 via xcode-select github action, as that guarantees we will get iOS 14.x simulators. For local execution of the tests you must have a side-by-side install of Xcode-12.5.1 (or Xcode-12.4 I guess) and use sudo xcode-select -s /Applications/Xcode-12.5.1.app to use it or iOS e2e will fail for you.. Here is a reasonable article explaining how to do it https://medium.com/@YogevSitton/install-xcode-7-and-xcode-8-side-by-side-5bf40ea8f5ac though you need to go to your Apple Account's downloads / all downloads section and select something beside Xcode 7 obviously

6- I took the opportunity to update all the other javascript dependencies and de-flake CI a little bit since this PR was forcing a bunch of compile / test cycles anyway and it was easy to qualify them

Test Plan

If E2E works in CI, this works


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Sep 22, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/AsPxQsVYZsw1FbtwQMGHBY4zSbE6
✅ Preview: https://react-native-firebase-git-mikehardy-detox-upgrade-invertase.vercel.app

[Deployment for 6f92b30 failed]

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/58S77tyL9WShjJboZp27rWxttpsk
✅ Preview: Canceled

[Deployment for 6f92b30 canceled]

@codecov
Copy link

codecov bot commented Sep 22, 2021

Codecov Report

Merging #5734 (be423e2) into master (c56bdb3) will decrease coverage by 0.58%.
The diff coverage is n/a.

❗ Current head be423e2 differs from pull request most recent head 6f92b30. Consider uploading reports for the commit 6f92b30 to get more accurate results

@@             Coverage Diff              @@
##             master    #5734      +/-   ##
============================================
- Coverage     53.71%   53.14%   -0.57%     
+ Complexity      632      620      -12     
============================================
  Files           208      208              
  Lines         10069    10069              
  Branches       1542     1542              
============================================
- Hits           5408     5350      -58     
- Misses         4377     4463      +86     
+ Partials        284      256      -28     

@mikehardy
Copy link
Collaborator Author

Okay, confirmed iOS 13.7 is not a valid OS specifier in CI. I dislike the OS specifier anyway, so this will either wait on Detox official iOS15 support or I'll hack in the same and post it for them. Probably wait - here's the URL to upstream issue wix/Detox#2895

@mikehardy
Copy link
Collaborator Author

also needs testing on an M1 mac, there was a note that Detox v18 may not work there

@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next September 24, 2021 20:43 Inactive
@mikehardy mikehardy changed the title tests(e2e): forward port to Detox test framework v18 test(e2e): forward port to Detox test framework v18 Sep 27, 2021
@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next September 29, 2021 19:19 Inactive
@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next September 29, 2021 20:27 Inactive
This is in prep for support of iOS15 in Detox
Until Detox supports iOS 15, we'll use macos-11 runner but we need
to pin Xcode to 12.5.1 so we get iOS 14 simulators for e2e tests

Cleanup from Detox update - should be merged with it

This just removes the os 13.7 designator for the detox config
that was going to be unworkable in practice, I'm pinning iOS builds
on Xcode 12.5.1 for now for Detox (though I build work projects with 13)
Hopefully the last RC, should clean even more things up
RC1 worked already so this should be low risk
Detox v18 was noted as being slower by some other collaborator in
Detox repo issues, it is apparently true. 10 minus results in test timeout
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Oct 1, 2021
@mikehardy mikehardy merged commit 27d8ad8 into master Oct 1, 2021
@mikehardy mikehardy deleted the @mikehardy/detox-upgrade branch October 1, 2021 17:21
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.

1 participant