-
Notifications
You must be signed in to change notification settings - Fork 20
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
Replace test app with react-native-test-app #120
Conversation
TODO: Fix the CI 😅 Going to do this tomorrow |
nice 🫡 let me know if I can help with the CI |
d9105f1
to
215616b
Compare
@okwasniewski silly question, does it support RN Web as well? Sorry I know nothing about this topic yet but it sounds very cool |
@wcandillon So unfortunately, there is no first class support for web in test-app: microsoft/react-native-test-app#812 I guess you could have two examples (test-app + expo) that would probably give you the best coverage |
I found an issue with the integration of test-app + turbo I've contacted @tido64 so we can come up with a solution. TLDR of the issue: When running However when executing this command through turbo (https://turbo.build/)
Making the Xcode build fail |
5b24dc2
to
da87a0a
Compare
Hey @wcandillon build is now green! Its ready to review |
haha no way :) |
silly question: can we easily build the app in the new arch? 😇 |
Yeah! For sure. For iOS:
For Android: Modify
We can also add new arch build to the CI later |
this is fantastic thank. Right now this doesn't work on iOS (Canvas API crashes, that's a bug on our side) nor Android (e2e not working, probably an issue on our side as well). However are currently fixing a serious bug in our Canvas API. What I will do on my side is to work on shipping this bug fixes asap so we can test it. But I promise I won't touch the example app so you don't need to fix time fixing conflicts and so on. Thank you again for this, this is very compelling. |
Awesome work, thanks @okwasniewski for taking the change and getting it ready and @tido64 for helping RNTA work with Turborepo repos! |
None of the examples work for me. First I thought it was related to some canvas API bugs we are currently fixing but now it looks like it might be something else? |
@wcandillon What is the error you are seeing? Im taking a look |
@wcandillon Im sorry but I can't reproduce this issue, everything works fine for me. Just to be sure you disabled the Metal API Validation in Xcode? CleanShot.2024-09-25.at.11.03.44.mp4 |
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.
this is big thank you for this.
@okwasniewski I'm experiencing some crashes on iOS, can you check on your side? |
@wcandillon Can you hint me what is crashing for you? I can see the build is failing but that's because of missing dawn.json file. |
the e2e tests but the same error happens when browsing the examples. I also cannot install the apk on Android but the issue might be on my side there. |
@wcandillon I'm going to test Android soon, so the tests were green on previous app and now they are red? Is it the setup of the tests or is it a native crash? I didn't run them |
It's a native crash that I can also reproduce by browsing the examples. |
I'll also try and test |
@wcandillon Sorry for the delay. I went to every example and no crash (I built using Android Studio) but it shouldn't matter. Can you provide a crash stack trace? Either way I don't think its related to TestApp more likely its the library. CleanShot.2024-09-27.at.15.55.39.mp4 |
Here is the list of issues/regressions I am experiencing. However, these probably shouldn't be called regressions since we are just changing the example app, so the module should still work there. On iOS, the This behavior is not happening in apps/paper, and I'm not sure why yet. |
@wcandillon is there any chance those issues are intermittent and not related to this PR? I can't think of why the test app would cause those things to fail, and I would personally like this PR to unblock new architecture / macOS / potential future Windows support. I just set up a new laptop so I'll try to repro the issues mentioned sometime this week on a clean install. |
I found the issue and it is as we expected unrelated to this PR, sorting it out now. |
@okwasniewski do you know how I can have the "Metal API Validation" unchecked in the example app? If I uncheck it in xcode, I don't see a change in git, probably because the file is in gitignore? |
@tido64 perhaps this is a feature request for RNTA? Otherwise maybe an expo config plugin? |
@okwasniewski I am also unable to build the app on fabric/android, my bad for not testing it before. |
You will have to write an Expo config plugin for this (or patch the More info on Config Plugins can be found here: https://github.com/microsoft/react-native-test-app/wiki/Config-Plugins |
In order to help maintenance and improve support for new out of tree platforms (like macOS and visionOS), let's replace the test app with https://github.com/microsoft/react-native-test-app . This simplifies the native code needed to stand up the example apps. In the case of apple platforms, it's now a single pod-sec.
Most of the work was done here: #109 by @Saadnajmi so kudos to him for that!