Add type-safe API for passing around benchmark results (#53143)

Summary:
Pull Request resolved: #53143

# Changelog:
[Internal] -

This refactors the way Fantom benchmark test results are passed to the top level, making it type safe and more maintainable.

Reviewed By: andrewdacenko

Differential Revision: D79812707

fbshipit-source-id: d8bfef7e1b0c11b277a08f5e4c810f8c1efd7f89

Author: rshest

private/react-native-fantom/runner/benchmarkUtils.js

@@ -8,52 +8,36 @@
  * @format
  */
 
-import {markdownTable} from './utils';
-
-type TestTaskTiming = {
-  name: string,
-  latency: {
-    mean: number,
-    min: number,
-    max: number,
-    p50: number,
-    p75: number,
-    p99: number,
-  },
-};
+import type {BenchmarkResult} from '../src/Benchmark';
 
-export type BenchmarkTestArtifact = {
-  type: string,
-  timings: $ReadOnlyArray<TestTaskTiming>,
-};
+import {markdownTable} from './utils';
 
 export const printBenchmarkResultsRanking = (
-  testResults: Array<{
+  benchmarkResults: Array<{
     title: string,
-    testArtifact: mixed,
+    result: BenchmarkResult,
   }>,
 ) => {
   const testTaskTimings: {[string]: {[string]: number}} = {};
   let numTestVariants = 0;
 
-  for (const testResult of testResults) {
-    // $FlowExpectedError[incompatible-cast]
-    const testArtifact = testResult?.testArtifact as ?BenchmarkTestArtifact;
+  for (const benchmarkResult of benchmarkResults) {
+    const result = benchmarkResult.result;
     if (
-      testArtifact == null ||
-      testArtifact.timings == null ||
-      testArtifact.type !== 'benchmark' ||
-      testResult.title == null
+      result == null ||
+      result.timings == null ||
+      benchmarkResult.title == null
     ) {
       continue;
     }
     numTestVariants++;
-    for (const taskTiming of testArtifact.timings) {
+    for (const taskTiming of result.timings) {
       const taskName = taskTiming.name;
       if (testTaskTimings[taskName] === undefined) {
         testTaskTimings[taskName] = {};
       }
-      testTaskTimings[taskName][testResult.title] = taskTiming.latency.p50;
+      testTaskTimings[taskName][benchmarkResult.title] =
+        taskTiming.latency?.p50 ?? taskTiming.latency.mean;
     }
   }
   if (numTestVariants <= 1 || Object.keys(testTaskTimings).length === 0) {

private/react-native-fantom/runner/runner.js

@@ -14,7 +14,7 @@ import type {
   TestSuiteResult,
 } from '../runtime/setup';
 import type {TestSnapshotResults} from '../runtime/snapshotContext';
-import type {BenchmarkTestArtifact} from './benchmarkUtils';
+import type {BenchmarkResult} from '../src/Benchmark';
 import type {
   AsyncCommandResult,
   ConsoleLogMessage,
@@ -80,7 +80,7 @@ function buildError(
 
 async function processRNTesterCommandResult(
   result: AsyncCommandResult,
-): Promise<TestSuiteResult> {
+): Promise<[TestSuiteResult, ?BenchmarkResult]> {
   const stdoutChunks = [];
   const stderrChunks = [];
 
@@ -92,7 +92,7 @@ async function processRNTesterCommandResult(
     stderrChunks.push(chunk);
   });
 
-  let testResult;
+  let testResult, benchmarkResult;
 
   const rl = readline.createInterface({input: result.childProcess.stdout});
   rl.on('line', (rawLine: string) => {
@@ -116,6 +116,9 @@ async function processRNTesterCommandResult(
       case 'test-result':
         testResult = parsed;
         break;
+      case 'benchmark-result':
+        benchmarkResult = parsed;
+        break;
       case 'console-log':
         printConsoleLog(parsed);
         break;
@@ -152,7 +155,7 @@ async function processRNTesterCommandResult(
     );
   }
 
-  return testResult;
+  return [testResult, benchmarkResult];
 }
 
 function generateBytecodeBundle({
@@ -223,6 +226,7 @@ module.exports = async function runTest(
   );
 
   const testResultsByConfig = [];
+  const benchmarkResults = [];
 
   const skippedTestResults = ({
     ancestorTitles,
@@ -241,7 +245,6 @@ module.exports = async function runTest(
       snapshotResults: {} as TestSnapshotResults,
       status: 'pending' as TestCaseResult['status'],
       testFilePath: testPath,
-      testArtifact: {} as mixed,
       title,
     },
   ];
@@ -371,9 +374,8 @@ module.exports = async function runTest(
           hermesVariant: testConfig.hermesVariant,
         });
 
-    const processedResult = await processRNTesterCommandResult(
-      rnTesterCommandResult,
-    );
+    const [processedResult, benchmarkResult] =
+      await processRNTesterCommandResult(rnTesterCommandResult);
 
     if (containsError(processedResult) || EnvironmentOptions.profileJS) {
       await createSourceMap({
@@ -439,6 +441,13 @@ module.exports = async function runTest(
       });
     }
 
+    if (benchmarkResult != null) {
+      benchmarkResults.push({
+        title: testResults[0]?.ancestorTitles?.[0] ?? maybeCommonAncestor,
+        result: benchmarkResult,
+      });
+    }
+
     testResultsByConfig.push(testResults);
   }
 
@@ -455,17 +464,7 @@ module.exports = async function runTest(
     snapshotResults,
   );
 
-  printBenchmarkResultsRanking(
-    testResults.map(testResult => {
-      // $FlowExpectedError[incompatible-cast]
-      const testArtifact = testResult.testArtifact as ?BenchmarkTestArtifact;
-      const title = testResult.ancestorTitles[0];
-      return {
-        title,
-        testArtifact,
-      };
-    }),
-  );
+  printBenchmarkResultsRanking(benchmarkResults);
 
   return {
     testFilePath: testPath,

private/react-native-fantom/runtime/setup.js

@@ -8,6 +8,7 @@
  * @format
  */
 
+import type {BenchmarkResult} from '../src/Benchmark';
 import type {SnapshotConfig, TestSnapshotResults} from './snapshotContext';
 
 import {getConstants} from '../src/Constants';
@@ -26,7 +27,6 @@ export type TestCaseResult = {
   failureDetails: Array<FailureDetail>,
   numPassingAsserts: number,
   snapshotResults: TestSnapshotResults,
-  testArtifact?: mixed,
   // location: string,
 };
 
@@ -315,7 +315,6 @@ function runSpec(spec: Spec): TestCaseResult {
     failureDetails: [],
     numPassingAsserts: 0,
     snapshotResults: {},
-    testArtifact: null,
   };
 
   if (!shouldRunSuite(spec)) {
@@ -330,7 +329,7 @@ function runSpec(spec: Spec): TestCaseResult {
 
   try {
     invokeHooks(spec.parentContext, 'beforeEachHooks');
-    result.testArtifact = spec.implementation();
+    spec.implementation();
     invokeHooks(spec.parentContext, 'afterEachHooks');
 
     status = 'passed';
@@ -402,6 +401,13 @@ function reportTestSuiteResult(testSuiteResult: TestSuiteResult): void {
   );
 }
 
+export function reportBenchmarkResult(result: BenchmarkResult): void {
+  // Force the import of the native module to be lazy
+  const NativeFantom =
+    require('react-native/src/private/testing/fantom/specs/NativeFantom').default;
+  NativeFantom.reportTestSuiteResultsJSON(JSON.stringify(result));
+}
+
 function validateEmptyMessageQueue(): void {
   // Force the import of the native module to be lazy
   const NativeFantom =

private/react-native-fantom/src/Benchmark.js

@@ -8,6 +8,7 @@
  * @format
  */
 
+import {reportBenchmarkResult} from '../runtime/setup';
 import {getConstants} from './index';
 import nullthrows from 'nullthrows';
 import NativeCPUTime from 'react-native/src/private/testing/fantom/specs/NativeCPUTime';
@@ -30,6 +31,23 @@ export type SuiteOptions = $ReadOnly<{
 
 export type TestOptions = FnOptions;
 
+export type TestTaskTiming = {
+  name: string,
+  latency: {
+    mean: number,
+    min: number,
+    max: number,
+    p50?: number,
+    p75?: number,
+    p99?: number,
+  },
+};
+
+export type BenchmarkResult = {
+  type: string,
+  timings: $ReadOnlyArray<TestTaskTiming>,
+};
+
 type InternalTestOptions = $ReadOnly<{
   ...FnOptions,
   only?: boolean,
@@ -176,7 +194,7 @@ export function suite(
         'Failing focused test to prevent it from being committed',
       );
     }
-    return createBenchmarkTestArtifact(bench, tasks);
+    reportBenchmarkResult(createBenchmarkResultsObject(bench, tasks));
   });
 
   const test = (
@@ -255,9 +273,12 @@ function printBenchmarkResults(bench: Bench) {
   console.log('');
 }
 
-function createBenchmarkTestArtifact(bench: Bench, tasks: Array<TestTask>) {
+function createBenchmarkResultsObject(
+  bench: Bench,
+  tasks: Array<TestTask>,
+): BenchmarkResult {
   return {
-    type: 'benchmark',
+    type: 'benchmark-result',
     timings: tasks.map((task, i) => {
       const result = bench.results[i];
       const {min, max, mean, p50, p75, p99} = result.latency;