From 7105a55e766e6e9fb02a92faef8f84b63cf9acda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 01:54:08 -0800 Subject: [PATCH 1/5] Follow naming convention for NativeFantom module Differential Revision: D67549203 --- .../src/getFantomRenderedOutput.js | 4 ++-- packages/react-native-fantom/src/index.js | 12 ++++++------ .../specs/{NativeFantomModule.js => NativeFantom.js} | 4 +++- 3 files changed, 11 insertions(+), 9 deletions(-) rename packages/react-native-fantom/src/specs/{NativeFantomModule.js => NativeFantom.js} (90%) diff --git a/packages/react-native-fantom/src/getFantomRenderedOutput.js b/packages/react-native-fantom/src/getFantomRenderedOutput.js index 496a462a3d32..45500fde1742 100644 --- a/packages/react-native-fantom/src/getFantomRenderedOutput.js +++ b/packages/react-native-fantom/src/getFantomRenderedOutput.js @@ -9,7 +9,7 @@ * @oncall react_native */ -import FantomModule from './specs/NativeFantomModule'; +import NativeFantom from './specs/NativeFantom'; // $FlowExpectedError[untyped-import] import micromatch from 'micromatch'; import * as React from 'react'; @@ -114,7 +114,7 @@ export default function getFantomRenderedOutput( } = config; return new FantomRenderedOutput( JSON.parse( - FantomModule.getRenderedOutput(surfaceId, { + NativeFantom.getRenderedOutput(surfaceId, { includeRoot, includeLayoutMetrics, }), diff --git a/packages/react-native-fantom/src/index.js b/packages/react-native-fantom/src/index.js index b765810f66a0..7b06e4e9a655 100644 --- a/packages/react-native-fantom/src/index.js +++ b/packages/react-native-fantom/src/index.js @@ -15,7 +15,7 @@ import type { import type {MixedElement} from 'react'; import getFantomRenderedOutput from './getFantomRenderedOutput'; -import FantomModule from './specs/NativeFantomModule'; +import NativeFantom from './specs/NativeFantom'; import ReactFabric from 'react-native/Libraries/Renderer/shims/ReactFabric'; let globalSurfaceIdCounter = 1; @@ -35,7 +35,7 @@ class Root { render(element: MixedElement) { if (!this.#hasRendered) { - FantomModule.startSurface(this.#surfaceId); + NativeFantom.startSurface(this.#surfaceId); this.#hasRendered = true; } @@ -43,13 +43,13 @@ class Root { } getMountingLogs(): Array { - return FantomModule.getMountingManagerLogs(this.#surfaceId); + return NativeFantom.getMountingManagerLogs(this.#surfaceId); } destroy() { // TODO: check for leaks. - FantomModule.stopSurface(this.#surfaceId); - FantomModule.flushMessageQueue(); + NativeFantom.stopSurface(this.#surfaceId); + NativeFantom.flushMessageQueue(); } getRenderedOutput(config: RenderOutputConfig = {}): FantomRenderedOutput { @@ -100,7 +100,7 @@ export function runWorkLoop(): void { try { flushingQueue = true; - FantomModule.flushMessageQueue(); + NativeFantom.flushMessageQueue(); } finally { flushingQueue = false; } diff --git a/packages/react-native-fantom/src/specs/NativeFantomModule.js b/packages/react-native-fantom/src/specs/NativeFantom.js similarity index 90% rename from packages/react-native-fantom/src/specs/NativeFantomModule.js rename to packages/react-native-fantom/src/specs/NativeFantom.js index da67f38a1e75..a2d2db9fc31c 100644 --- a/packages/react-native-fantom/src/specs/NativeFantomModule.js +++ b/packages/react-native-fantom/src/specs/NativeFantom.js @@ -26,4 +26,6 @@ interface Spec extends TurboModule { getRenderedOutput: (surfaceId: number, config: RenderFormatOptions) => string; } -export default TurboModuleRegistry.getEnforcing('Fantom') as Spec; +export default TurboModuleRegistry.getEnforcing( + 'NativeFantomCxx', +) as Spec; From f621c11abe931d1f729fb5bbccc39c97f496ec1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 01:54:08 -0800 Subject: [PATCH 2/5] Use codegen for Fantom native module Differential Revision: D67759729 --- packages/react-native-fantom/src/getFantomRenderedOutput.js | 2 +- packages/react-native-fantom/src/index.js | 2 +- .../src/private/specs/modules}/NativeFantom.js | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) rename packages/{react-native-fantom/src/specs => react-native/src/private/specs/modules}/NativeFantom.js (79%) diff --git a/packages/react-native-fantom/src/getFantomRenderedOutput.js b/packages/react-native-fantom/src/getFantomRenderedOutput.js index 45500fde1742..2e7f36cc9a3d 100644 --- a/packages/react-native-fantom/src/getFantomRenderedOutput.js +++ b/packages/react-native-fantom/src/getFantomRenderedOutput.js @@ -9,10 +9,10 @@ * @oncall react_native */ -import NativeFantom from './specs/NativeFantom'; // $FlowExpectedError[untyped-import] import micromatch from 'micromatch'; import * as React from 'react'; +import NativeFantom from 'react-native/src/private/specs/modules/NativeFantom'; export type RenderOutputConfig = { ...FantomRenderedOutputConfig, diff --git a/packages/react-native-fantom/src/index.js b/packages/react-native-fantom/src/index.js index 7b06e4e9a655..67795cc5c67f 100644 --- a/packages/react-native-fantom/src/index.js +++ b/packages/react-native-fantom/src/index.js @@ -15,8 +15,8 @@ import type { import type {MixedElement} from 'react'; import getFantomRenderedOutput from './getFantomRenderedOutput'; -import NativeFantom from './specs/NativeFantom'; import ReactFabric from 'react-native/Libraries/Renderer/shims/ReactFabric'; +import NativeFantom from 'react-native/src/private/specs/modules/NativeFantom'; let globalSurfaceIdCounter = 1; diff --git a/packages/react-native-fantom/src/specs/NativeFantom.js b/packages/react-native/src/private/specs/modules/NativeFantom.js similarity index 79% rename from packages/react-native-fantom/src/specs/NativeFantom.js rename to packages/react-native/src/private/specs/modules/NativeFantom.js index a2d2db9fc31c..34c04f2eaebf 100644 --- a/packages/react-native-fantom/src/specs/NativeFantom.js +++ b/packages/react-native/src/private/specs/modules/NativeFantom.js @@ -4,13 +4,13 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @flow strict-local + * @flow strict * @format */ -import type {TurboModule} from 'react-native/Libraries/TurboModule/RCTExport'; +import type {TurboModule} from '../../../../Libraries/TurboModule/RCTExport'; -import {TurboModuleRegistry} from 'react-native'; +import * as TurboModuleRegistry from '../../../../Libraries/TurboModule/TurboModuleRegistry'; // match RenderFormatOptions.h export type RenderFormatOptions = { From 6a9d1acd68caf1d21a65d3ccc4fe6b882f0ae1d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 01:54:08 -0800 Subject: [PATCH 3/5] Remove existing logs and warnings from tests Differential Revision: D67602299 --- .../src/__tests__/Fantom-itest.js | 68 ++++++++++++------- .../src/getFantomRenderedOutput.js | 11 ++- .../__tests__/IntersectionObserver-itest.js | 32 +++------ .../__tests__/MutationObserver-itest.js | 16 ++--- 4 files changed, 69 insertions(+), 58 deletions(-) diff --git a/packages/react-native-fantom/src/__tests__/Fantom-itest.js b/packages/react-native-fantom/src/__tests__/Fantom-itest.js index f16d30bd10fb..250c31d4d828 100644 --- a/packages/react-native-fantom/src/__tests__/Fantom-itest.js +++ b/packages/react-native-fantom/src/__tests__/Fantom-itest.js @@ -76,28 +76,30 @@ describe('Fantom', () => { // TODO: when error handling is fixed, this should verify using `toThrow` it('should throw when running a task inside another task', () => { - let lastCallbackExecuted = 0; + let threw = false; + runTask(() => { - lastCallbackExecuted = 1; - runTask(() => { - lastCallbackExecuted = 2; - throw new Error('Recursive runTask should be unreachable'); - }); + // TODO replace with expect(() => { ... }).toThrow() when error handling is fixed + try { + runTask(() => {}); + } catch { + threw = true; + } }); - expect(lastCallbackExecuted).toBe(1); + expect(threw).toBe(true); + + threw = false; runTask(() => { queueMicrotask(() => { - lastCallbackExecuted = 3; - runTask(() => { - lastCallbackExecuted = 4; - throw new Error( - 'Recursive runTask from micro-task should be unreachable', - ); - }); + try { + runTask(() => {}); + } catch { + threw = true; + } }); }); - expect(lastCallbackExecuted).toBe(3); + expect(threw).toBe(true); }); }); @@ -125,16 +127,24 @@ describe('Fantom', () => { runTask(() => { root.render( <> - - + + , ); }); expect(root.getRenderedOutput().toJSX()).toEqual( <> - - + + , ); @@ -233,18 +243,26 @@ describe('Fantom', () => { runTask(() => { root.render( <> - - hello world! - + + hello world! + , ); }); expect(root.getRenderedOutput({props: []}).toJSX()).toEqual( <> - - hello world! - + + hello world! + , ); diff --git a/packages/react-native-fantom/src/getFantomRenderedOutput.js b/packages/react-native-fantom/src/getFantomRenderedOutput.js index 2e7f36cc9a3d..ede6e38285fb 100644 --- a/packages/react-native-fantom/src/getFantomRenderedOutput.js +++ b/packages/react-native-fantom/src/getFantomRenderedOutput.js @@ -152,16 +152,20 @@ function convertRawJsonToJSX( function createJSXElementForTestComparison( type: string, props: mixed, + key?: ?string, ): React.Node { const Tag = type; - return ; + return ; } function rnTypeToTestType(type: string): string { return `rn-${type.substring(0, 1).toLowerCase() + type.substring(1)}`; } -function jsonChildToJSXChild(jsonChild: FantomJsonObject | string): React.Node { +function jsonChildToJSXChild( + jsonChild: FantomJsonObject | string, + index?: ?number, +): React.Node { if (typeof jsonChild === 'string') { return jsonChild; } else { @@ -172,6 +176,7 @@ function jsonChildToJSXChild(jsonChild: FantomJsonObject | string): React.Node { jsxChildren == null ? jsonChild.props : {...jsonChild.props, children: jsxChildren}, + index != null ? String(index) : undefined, ); } } @@ -184,7 +189,7 @@ function jsonChildrenToJSXChildren(jsonChildren: FantomJsonObject['children']) { let allJSXChildrenAreStrings = true; let jsxChildrenString = ''; for (let i = 0; i < jsonChildren.length; i++) { - const jsxChild = jsonChildToJSXChild(jsonChildren[i]); + const jsxChild = jsonChildToJSXChild(jsonChildren[i], i); jsxChildren.push(jsxChild); if (allJSXChildrenAreStrings) { if (typeof jsxChild === 'string') { diff --git a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js index a7ea8e84e8dd..8ad459de8af1 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js @@ -487,7 +487,6 @@ describe('IntersectionObserver', () => { maybeNode = receivedNode; }} /> - , , ); }); @@ -549,7 +548,6 @@ describe('IntersectionObserver', () => { maybeNode = receivedNode; }} /> - , , ); }); @@ -610,7 +608,6 @@ describe('IntersectionObserver', () => { maybeNode = receivedNode; }} /> - , , ); }); @@ -942,7 +939,6 @@ describe('IntersectionObserver', () => { maybeNode = receivedNode; }} /> - , , ); }); @@ -1003,7 +999,6 @@ describe('IntersectionObserver', () => { maybeNode = receivedNode; }} /> - , , ); }); @@ -1314,13 +1309,9 @@ describe('IntersectionObserver', () => { }); expect(node.isConnected).toBe(false); - Fantom.runTask(() => { - observer = new IntersectionObserver(() => {}); - observer.observe(node); - // TODO what happens if this throws an exception? - observer.unobserve(node); - throw new Error('unobserve should not throw'); - }); + observer = new IntersectionObserver(() => {}); + observer.observe(node); + observer.unobserve(node); }); it('should not report the initial state if the target is unobserved before it is delivered', () => { @@ -1497,19 +1488,16 @@ describe('IntersectionObserver', () => { const node = ensureReactNativeElement(maybeNode); - Fantom.runTask(() => { - observer1 = new IntersectionObserver(() => {}); - observer2 = new IntersectionObserver(() => {}); + observer1 = new IntersectionObserver(() => {}); + observer2 = new IntersectionObserver(() => {}); - observer1.observe(node); - observer2.observe(node); + observer1.observe(node); + observer2.observe(node); - observer1.unobserve(node); + observer1.unobserve(node); - // The second call shouldn't log errors (that would make the test fail). - observer2.unobserve(node); - throw new Error('unobserve should not throw'); - }); + // The second call shouldn't log errors (that would make the test fail). + observer2.unobserve(node); }); }); diff --git a/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js b/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js index 25801da127c9..8163f88a15a5 100644 --- a/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js +++ b/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js @@ -383,14 +383,14 @@ describe('MutationObserver', () => { { - maybeObservedNode = ensureReactNativeElement(receivedNode); + maybeObservedNode = receivedNode; }}> , ); }); - const observedNode = nullthrows(maybeObservedNode); + const observedNode = ensureReactNativeElement(maybeObservedNode); const observerCallback = jest.fn(); const observer = new MutationObserver(observerCallback); @@ -968,13 +968,13 @@ describe('MutationObserver', () => { { - maybeObservedNode = ensureReactNativeElement(receivedNode); + maybeObservedNode = receivedNode; }} />, ); }); - const observedNode = nullthrows(maybeObservedNode); + const observedNode = ensureReactNativeElement(maybeObservedNode); const observerCallback = jest.fn(); const observer = new MutationObserver(observerCallback); @@ -1016,13 +1016,13 @@ describe('MutationObserver', () => { { - maybeObservedNode = ensureReactNativeElement(receivedNode); + maybeObservedNode = receivedNode; }} />, ); }); - const observedNode = nullthrows(maybeObservedNode); + const observedNode = ensureReactNativeElement(maybeObservedNode); const observerCallback = jest.fn(); const observer = new MutationObserver(observerCallback); @@ -1048,13 +1048,13 @@ describe('MutationObserver', () => { { - maybeObservedNode = ensureReactNativeElement(receivedNode); + maybeObservedNode = receivedNode; }} />, ); }); - const observedNode = nullthrows(maybeObservedNode); + const observedNode = ensureReactNativeElement(maybeObservedNode); Fantom.runTask(() => { root.render(<>); From b836df86f754c030c18292d044c1506e9d3bbd45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 01:54:08 -0800 Subject: [PATCH 4/5] Remove unnecessary filter for AppRegistry logs Differential Revision: D67600610 --- packages/react-native-fantom/runner/runner.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/react-native-fantom/runner/runner.js b/packages/react-native-fantom/runner/runner.js index 776ee43957b3..f321e64ac117 100644 --- a/packages/react-native-fantom/runner/runner.js +++ b/packages/react-native-fantom/runner/runner.js @@ -47,10 +47,7 @@ function parseRNTesterCommandResult(result: ReturnType): { } { const stdout = result.stdout.toString(); - const outputArray = stdout - .trim() - .split('\n') - .filter(log => !log.startsWith('Running "')); // remove AppRegistry logs. + const outputArray = stdout.trim().split('\n'); // The last line should be the test output in JSON format const testResultJSON = outputArray.pop(); From c2d32192fb11c4d207c5a4cf31cad6603acf6ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 03:23:37 -0800 Subject: [PATCH 5/5] Use structured output for Fantom logs and test results (#48369) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/48369 Changelog: [internal] This improves logs in Fantom by preserving the original log levels. It's also in preparation for adding support for log streaming in tests. Reviewed By: javache Differential Revision: D67600616 --- packages/react-native-fantom/runner/runner.js | 47 ++++++++++++++----- packages/react-native-fantom/runner/utils.js | 28 +++++++++++ packages/react-native-fantom/runtime/setup.js | 8 +++- .../src/private/specs/modules/NativeFantom.js | 1 + 4 files changed, 72 insertions(+), 12 deletions(-) diff --git a/packages/react-native-fantom/runner/runner.js b/packages/react-native-fantom/runner/runner.js index f321e64ac117..74c24e82126f 100644 --- a/packages/react-native-fantom/runner/runner.js +++ b/packages/react-native-fantom/runner/runner.js @@ -10,6 +10,7 @@ */ import type {TestSuiteResult} from '../runtime/setup'; +import type {ConsoleLogMessage} from './utils'; import entrypointTemplate from './entrypoint-template'; import getFantomTestConfig from './getFantomTestConfig'; @@ -22,6 +23,7 @@ import { getBuckModesForPlatform, getDebugInfoFromCommandResult, getShortHash, + printConsoleLogs, runBuck2, symbolicateStackTrace, } from './utils'; @@ -42,27 +44,50 @@ const BUILD_OUTPUT_PATH = fs.mkdtempSync( const PRINT_FANTOM_OUTPUT: false = false; function parseRNTesterCommandResult(result: ReturnType): { - logs: string, + logs: $ReadOnlyArray, testResult: TestSuiteResult, } { const stdout = result.stdout.toString(); - const outputArray = stdout.trim().split('\n'); + const logs = []; + let testResult; - // The last line should be the test output in JSON format - const testResultJSON = outputArray.pop(); + const lines = stdout + .split('\n') + .map(line => line.trim()) + .filter(Boolean); + + for (const line of lines) { + let parsed; + try { + parsed = JSON.parse(line); + } catch {} + + switch (parsed?.type) { + case 'test-result': + testResult = parsed; + break; + case 'console-log': + logs.push(parsed); + break; + default: + logs.push({ + type: 'console-log', + message: line, + level: 'info', + }); + break; + } + } - let testResult; - try { - testResult = JSON.parse(nullthrows(testResultJSON)); - } catch (error) { + if (testResult == null) { throw new Error( - 'Failed to parse test results from RN tester binary result.\n' + + 'Failed to find test results in RN tester binary output.\n' + getDebugInfoFromCommandResult(result), ); } - return {logs: outputArray.join('\n'), testResult}; + return {logs, testResult}; } function generateBytecodeBundle({ @@ -216,7 +241,7 @@ module.exports = async function runTest( const endTime = Date.now(); if (process.env.SANDCASTLE == null) { - console.log(rnTesterParsedOutput.logs); + printConsoleLogs(rnTesterParsedOutput.logs); } const testResults = diff --git a/packages/react-native-fantom/runner/utils.js b/packages/react-native-fantom/runner/utils.js index d5aa6deeac7a..97a486dcebaf 100644 --- a/packages/react-native-fantom/runner/utils.js +++ b/packages/react-native-fantom/runner/utils.js @@ -134,3 +134,31 @@ export function symbolicateStackTrace( }) .join('\n'); } + +export type ConsoleLogMessage = { + type: 'console-log', + level: 'info' | 'warn' | 'error', + message: string, +}; + +export function printConsoleLogs( + logs: $ReadOnlyArray, +): void { + for (const log of logs) { + switch (log.type) { + case 'console-log': + switch (log.level) { + case 'info': + console.log(log.message); + break; + case 'warn': + console.warn(log.message); + break; + case 'error': + console.error(log.message); + break; + } + break; + } + } +} diff --git a/packages/react-native-fantom/runtime/setup.js b/packages/react-native-fantom/runtime/setup.js index 31055e4a9d4a..740f6e20288e 100644 --- a/packages/react-native-fantom/runtime/setup.js +++ b/packages/react-native-fantom/runtime/setup.js @@ -15,6 +15,7 @@ import expect from './expect'; import {createMockFunction} from './mocks'; import {setupSnapshotConfig, snapshotContext} from './snapshotContext'; import nullthrows from 'nullthrows'; +import NativeFantom from 'react-native/src/private/specs/modules/NativeFantom'; export type TestCaseResult = { ancestorTitles: Array, @@ -190,7 +191,12 @@ function executeTests() { } function reportTestSuiteResult(testSuiteResult: TestSuiteResult): void { - console.log(JSON.stringify(testSuiteResult)); + NativeFantom.reportTestSuiteResultsJSON( + JSON.stringify({ + type: 'test-result', + ...testSuiteResult, + }), + ); } global.$$RunTests$$ = () => { diff --git a/packages/react-native/src/private/specs/modules/NativeFantom.js b/packages/react-native/src/private/specs/modules/NativeFantom.js index 34c04f2eaebf..57e8f7fda028 100644 --- a/packages/react-native/src/private/specs/modules/NativeFantom.js +++ b/packages/react-native/src/private/specs/modules/NativeFantom.js @@ -24,6 +24,7 @@ interface Spec extends TurboModule { getMountingManagerLogs: (surfaceId: number) => Array; flushMessageQueue: () => void; getRenderedOutput: (surfaceId: number, config: RenderFormatOptions) => string; + reportTestSuiteResultsJSON: (results: string) => void; } export default TurboModuleRegistry.getEnforcing(