Skip to content

Conversation

tom-un
Copy link
Collaborator

@tom-un tom-un commented May 10, 2020

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

The macOS and iOS integration tests suite needs a test that loads each of the Example pages in RNTester. When we merge changes from upstream, we often miss the fact that pages in RNTester throw RedBox errors. This a problem upstream as well: Facebook master often has RNTester pages that RedBox when opened. This fix will be contributed upstream as well.

The RNTesterLoadAllPages is an XCTestCase sublcass that overrides the defaultTestSuite class method. In this method it run uses the RCTTestRunner to run a new JavaScript "test" in RNTester.ios.js called EnumerateExamplePages. This JavaScript test simply writes to console the key names of each test page in the RNTesterList.ios.js map. In defaultTestSuite a bridge log block function captures the console and builds up an array of the test names. It creates an XCTestCase for each test and returns the tests. The method for dynamically creating the tests at runtime was borrowed from GTMGoogleTestRunner.

When each dynamically created XCTestCase is executed, its selector also uses RCTTestRunner to run an RNTester test named "LoadPageTest_"<key>. In RNTesterApp.ios.js instances of a new component LoadPageTest is created for each RNTester page and registered using the scheme "LoadPageTest_"<key> in a way similar to how the existing Snapshot tests are registered.

Some RNTester pages cannot be tested in this manner: some pages require native modules that only function in the full RNTester app (these pages have their own dedicated integration test code), don't function in the iOS Simulator, or have other issues. A new skipTest key is added the RNTesterTypes to allow individual tests to be skipped by providing a "reason" string.

The NewAppScreenExample.js page has a RedBox error because it has a List where each item doesn't have a unique key prop. This was trivial to fix.

Several tests fail on macOS due to an issue in RCTUITextView. Since the RN .61 merge an exception can be thrown when nil is passed to NSTextStorage setAttributedString: -- "-[NSBigMutableString replaceCharactersInRange:withString:]: nil argument". This is avoided by assigning an empty [NSAttributedString] when the passed in string is nil.

This change also deforks the RNTesterApp.macos.js and RNTesterList.macos.js to simply use the ios version.

Changelog

[iOS] [Added] - Create a RNTesterLoadAllPages integration test

Test Plan

Extensive manual testing of all the RNTester pages on macOS and iOS.

Microsoft Reviewers: Open in CodeFlow

tom-un and others added 30 commits April 3, 2020 20:53
@tom-un tom-un requested a review from HeyImChris May 10, 2020 23:18
@tom-un tom-un merged commit 67685f4 into microsoft:master May 11, 2020
Saadnajmi added a commit that referenced this pull request Aug 9, 2024
Picking up from
#2145, many thanks
to @FalseLobster for the bulk of the work :)

Merge up to 0.74 branch cut
(facebook@c99d96b)

This PR combines most of the work into the single large merge commit for
a couple of reasons:
1. To preserve git history with upstream React Native, we cannot "squash
or rebase the commits. Therefore, we must do a merge commit.
2. I am personally not a fan of committing git merge markers and
resolving them in followup commits.

I did have a couple of extra commits after the merge commit for work
that I feel could be separated out. I will do my best to list
interesting changes in this merge, and what files to look at + what
changes I made in those files. For a more "exhaustive" list, I also
listed all files that had merge conflicts below. This isn't 100% of the
changes (I touched some extra files for some lint fixes / shimming of
`UIView` -> `RCTPlatformView` / `RCTUIView`) but it will get you a good
80-90%.

## Summary:

### Interesting upstream changes:

Each of these changes resulted in extra work to support macOS. See the
original commit, which files were touched, and then how those files are
diffed in this change.

-
facebook@1b85ed9
Moved RCTPushNotificationManager to use UNNotification APIs
-
facebook@3f621df
RCTActionSheetManager was reworked for synchronous calls
-
facebook@e2eb26c
RCTRedBox was reworked for iPad and orientation changes, the
functionality of RCTRedBoxWindow was reworked into the new
ViewController introduced upstream, RCTRedBoxController
-
facebook@0806ad7
Introduced a new stable id for the inspector dev server based on UUID
which required a custom macOS implementation
-
facebook@4c108aa
Added support to dynamic colors in Fabric

### Other interesting changes:
- Removed a lot of diffs around adding a `scaleFactor` to places where
we manually calculate frames / sizes. With React Native 0.74, [Yoga 3.0
introduces Per-Node
`PointScaleFactor`](#356),
which is probably more accurate than the `scale` prop we added to
RCTShadowView that keys off the main screen.
- Reverted #356.
This change added an integratiion test, and introduced a bunch of JS
diffs. The test itself has since been disabled, many of the JS diffs
were actually redundant (applying background colors in macOS only diffs
that got overwritten by new styling that upstream changes added, etc),
the test was not ported to new platforms (visionOS), and Integration
tests in general are heading a different direction (Appium +
WebdriverIO). I figured let's revert this and the diffs it introduced
rather than fix the integration test
- I had to manually ifdef out the dev menu `PerfMonitor` option as we
haven't ported that to macOS yet. This presumably now because a problem
as recent changes to bridgeless + removing diffs has changed which
modules are loaded when. In this case, I think the PerfMonitor module is
lazy loaded when you create the dev menu. Without the ifdef, we would
Redbox every time we open the dev menu.
- New lint rules did re-order a lot of imports in macOS specific
packages like our local-cli. I also did my best to move macOS specific
imports in JS files to a new line to make the diffs a little cleaner.
- I had to implement `RCTUITextView.selectedTextRange` on macOS, as that
started erroring in RNTester.

Todo:
- [ ] Publish new version of virtualized-list package

## Test Plan:

CI should pass. I encourage reviewers to check out and build RNTester as
well.

## List of merge conflicts.

```
Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   Gemfile.lock
	both modified:   packages/community-cli-plugin/package.json
	both modified:   packages/metro-config/package.json
	both modified:   packages/normalize-color/package.json
	both modified:   packages/react-native/Libraries/Alert/NativeAlertManager.js
	both modified:   packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h
	both modified:   packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm
	both modified:   packages/react-native/Libraries/Components/AccessibilityInfo/NativeAccessibilityManager.js
	deleted by them: packages/react-native/Libraries/Components/Button.flow.js
	both modified:   packages/react-native/Libraries/Components/Button.js
	both modified:   packages/react-native/Libraries/Components/ScrollView/ScrollView.js
	both modified:   packages/react-native/Libraries/Core/NativeExceptionsManager.js
	both modified:   packages/react-native/Libraries/Image/Image.ios.js
	both modified:   packages/react-native/Libraries/Image/RCTImageLoader.mm
	both modified:   packages/react-native/Libraries/NativeModules/specs/NativeDevSettings.js
	both modified:   packages/react-native/Libraries/PushNotificationIOS/NativePushNotificationManagerIOS.js
	both modified:   packages/react-native/Libraries/PushNotificationIOS/RCTPushNotificationManager.h
	both modified:   packages/react-native/Libraries/PushNotificationIOS/RCTPushNotificationManager.mm
	both modified:   packages/react-native/Libraries/Text/Text/RCTTextView.mm
	both modified:   packages/react-native/Libraries/Text/TextInput/RCTInputAccessoryViewManager.mm
	both modified:   packages/react-native/React-Core.podspec
	both modified:   packages/react-native/React/Base/RCTUtils.m
	both modified:   packages/react-native/React/CoreModules/RCTActionSheetManager.mm
	both modified:   packages/react-native/React/CoreModules/RCTAlertController.mm
	both modified:   packages/react-native/React/CoreModules/RCTAlertManager.mm
	both modified:   packages/react-native/React/CoreModules/RCTAppearance.mm
	both modified:   packages/react-native/React/CoreModules/RCTDevLoadingView.mm
	both modified:   packages/react-native/React/CoreModules/RCTDeviceInfo.mm
	both modified:   packages/react-native/React/CoreModules/RCTRedBox.mm
	both modified:   packages/react-native/React/CoreModules/RCTStatusBarManager.mm
	both modified:   packages/react-native/React/CoreModules/RCTTiming.mm
	both modified:   packages/react-native/React/CoreModules/React-CoreModules.podspec
	both modified:   packages/react-native/React/DevSupport/RCTInspectorDevServerHelper.mm
	both modified:   packages/react-native/React/Fabric/Mounting/ComponentViews/LegacyViewManagerInterop/RCTLegacyViewManagerInteropComponentView.mm
	both modified:   packages/react-native/React/Fabric/Mounting/ComponentViews/Modal/RCTModalHostViewComponentView.mm
	both modified:   packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm
	both modified:   packages/react-native/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
	both modified:   packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm
	both modified:   packages/react-native/React/Fabric/RCTConversions.h
	both modified:   packages/react-native/React/Fabric/RCTSurfacePointerHandler.mm
	both modified:   packages/react-native/React/Fabric/RCTSurfacePresenter.mm
	both modified:   packages/react-native/React/Modules/RCTUIManager.m
	both modified:   packages/react-native/React/React-RCTFabric.podspec
	both modified:   packages/react-native/React/UIUtils/RCTUIUtils.m
	both modified:   packages/react-native/React/Views/RCTDebuggingOverlayManager.h
	both modified:   packages/react-native/React/Views/RCTModalHostViewController.m
	both modified:   packages/react-native/React/Views/RCTShadowView.m
	both modified:   packages/react-native/React/Views/ScrollView/RCTScrollViewManager.m
	both modified:   packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.h
	both modified:   packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.mm
	both modified:   packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h
	both modified:   packages/react-native/ReactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/RCTPlatformColorUtils.mm
	deleted by them: packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/NSTextStorage+FontScaling.h
	both modified:   packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextPrimitivesConversions.h
	both modified:   packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp
	both modified:   packages/react-native/index.js
	both modified:   packages/react-native/package.json
	both modified:   packages/react-native/react-native.config.js
	both modified:   packages/react-native/scripts/cocoapods/helpers.rb
	both modified:   packages/react-native/scripts/cocoapods/jsengine.rb
	both modified:   packages/react-native/scripts/cocoapods/utils.rb
	both modified:   packages/react-native/third-party-podspecs/boost.podspec
	both modified:   packages/rn-tester/IntegrationTests/LayoutEventsTest.js
	both modified:   packages/rn-tester/NativeComponentExample/ios/RNTMyLegacyNativeViewManager.mm
	both modified:   packages/rn-tester/NativeComponentExample/ios/RNTMyNativeViewComponentView.h
	both modified:   packages/rn-tester/NativeComponentExample/ios/RNTMyNativeViewComponentView.mm
	both modified:   packages/rn-tester/NativeComponentExample/ios/RNTMyNativeViewManager.mm
	both modified:   packages/rn-tester/Podfile
	both modified:   packages/rn-tester/Podfile.lock
	both modified:   packages/rn-tester/RCTTest/FBSnapshotTestCase/FBSnapshotTestController.m
	both modified:   packages/rn-tester/RCTTest/FBSnapshotTestCase/UIImage+Compare.m
	both modified:   packages/rn-tester/RNTester/AppDelegate.mm
	deleted by them: packages/rn-tester/RNTester/RNTester.entitlements
	both modified:   packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj
	both modified:   packages/rn-tester/js/RNTesterApp.ios.js
	both modified:   packages/rn-tester/js/components/RNTesterBlock.js
	both modified:   packages/rn-tester/js/components/RNTesterModuleList.js
	both modified:   packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js
	both modified:   packages/rn-tester/js/examples/Image/ImageExample.js
	both modified:   packages/rn-tester/js/examples/PlatformColor/PlatformColorExample.js
	both modified:   packages/rn-tester/js/examples/TextInput/TextInputExample.ios.js
	both modified:   packages/rn-tester/js/examples/TextInput/TextInputSharedExamples.js
	both modified:   packages/virtualized-lists/Lists/VirtualizedList.js
	both modified:   packages/virtualized-lists/Lists/VirtualizedListCellRenderer.js
	both modified:   packages/virtualized-lists/package.json
	deleted by them: scripts/__tests__/publish-npm-test.js
	deleted by them: scripts/monorepo/get-and-update-nightlies.js
	deleted by them: scripts/prepare-package-for-release.js
	deleted by them: scripts/publish-npm.js
	both modified:   scripts/releases/utils/release-utils.js
	both modified:   yarn.lock
```
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.

3 participants