-
Notifications
You must be signed in to change notification settings - Fork 149
Add memory management and concurrency tests plans to our fork of Reac… #186
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
Conversation
Do we plan to submit this change to the FB repo? |
"RNTesterIntegrationTests\/testAccessibilityManagerTest", | ||
"RNTesterIntegrationTests\/testAppEventsTest", | ||
"RNTesterIntegrationTests\/testSyncMethodTest", | ||
"RNTesterIntegrationTests\/testWebSocketTest" |
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.
Do you have a sense of the problem? Maybe it's easy to just fix?
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.
I'm missing from the diff/commit notes, but what exactly did we change so these could be re-enabled?
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.
Pushed them up while they were still failing. Tom pushed a header fix today that stopped the inconsistent failures
Eventually it'd be great to. Short term we're still aiming to just get the proper testing infrastructure up and running, but when we finally merge our branch back into FB's, this should go too. |
…ministic errors can occur in the header maps of RCTText. Fix by forward declaring @protocols in headers, and #import the necessary headers in the .m files that need them.
… RunLoop has executed a block during shutdown, which results in this->m_shutdown being deferenced to deallocated memory. Capture a shared_ptr to this instead of a raw `this` in the lambdas.
Summary: Changelog: [Internal] - Update `react-native/debugger-frontend` from 35c4630...51a91a2 Resyncs `react-native/debugger-frontend` from GitHub - see `rn-chrome-devtools-frontend` [changelog](facebook/react-native-devtools-frontend@35c4630...51a91a2). ### Changelog | Commit | Author | Date/Time | Subject | | ------ | ------ | --------- | ------- | | [51a91a2ad](facebook/react-native-devtools-frontend@51a91a2ad) | Ruslan Lesiutin (28902667+hoxyq@users.noreply.github.com) | 2025-07-04T14:04:02+01:00 | [bump: react-devtools@6.1.4 (microsoft#189)](facebook/react-native-devtools-frontend@51a91a2ad) | | [761b96907](facebook/react-native-devtools-frontend@761b96907) | Vitali Zaidman (vzaidman@gmail.com) | 2025-07-04T12:55:39+01:00 | [fixed missing new line before string error causes (microsoft#186)](facebook/react-native-devtools-frontend@761b96907) | | [7774c38db](facebook/react-native-devtools-frontend@7774c38db) | Alex Hunt (hello@alexhunt.dev) | 2025-06-30T13:20:49+01:00 | [Disable request initiator panel in NetworkItemView (microsoft#185)](facebook/react-native-devtools-frontend@7774c38db) | Reviewed By: huntie Differential Revision: D77795834 fbshipit-source-id: 3c30f8e87593687415538902734b09b3144b70a4
…t Native
Please select one of the following
React Native has limited tooling running as it executes tests. We should perform more rigorous memory management checks given the memory issues we've seen in the past with RN. While we're at it, let's add a test plan for code concurrency. RN has a lot of multithreadedness that should have race conditions and execution various orders verified.
We should enable TSan as well, but right now so many tests crash with TSan on, it makes more sense to leave it off for now and then go through the tests one at a time and get them working with TSan before turning it on permanently.
Focus areas to test
Only impacts our tests.
Microsoft Reviewers: Open in CodeFlow