[WIP] fix: E2E performance pipeline flakyness and other improvements

.github/workflows/e2ePerformanceTests.yml

@@ -223,7 +223,7 @@ jobs:
         run: |
           if ls "./Host_Machine_Files/\$WORKING_DIRECTORY"/output2.md 1> /dev/null 2>&1; then
             # Print all the split files
-            for file in "./Host_Machine_Files/\$WORKING_DIRECTORY/output"*; do
+            for file in $(ls "./Host_Machine_Files/\$WORKING_DIRECTORY/output"* | sort -V); do
               if [ -f "$file" ]; then
                 cat "$file"
               fi
@@ -244,7 +244,7 @@ jobs:
             # Check if there are any split files
             if ls "./Host_Machine_Files/\$WORKING_DIRECTORY"/output2.md 1> /dev/null 2>&1; then
               # Post each split file as a separate comment
-              for file in "./Host_Machine_Files/\$WORKING_DIRECTORY/output"*; do
+              for file in $(ls "./Host_Machine_Files/\$WORKING_DIRECTORY/output"* | sort -V); do
                 if [ -f "$file" ]; then
                   gh pr comment ${{ inputs.PR_NUMBER }} -F "$file"
                 fi

src/libs/E2E/client.ts

@@ -1,7 +1,7 @@
 import Config from '../../../tests/e2e/config';
 import Routes from '../../../tests/e2e/server/routes';
 import type {NetworkCacheMap, TestConfig, TestResult} from './types';
-import {waitForActiveRequestsToBeEmpty} from './utils/NetworkInterceptor';
+import {originalFetch, waitForActiveRequestsToBeEmpty} from './utils/NetworkInterceptor';
 
 type NativeCommandPayload = {
     text: string;
@@ -24,7 +24,7 @@ const defaultRequestInit: RequestInit = {
 };
 
 const sendRequest = (url: string, data: Record<string, unknown>): Promise<Response> => {
-    return fetch(url, {
+    return originalFetch(url, {
         method: 'POST',
         headers: {
             // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -59,12 +59,12 @@ const submitTestResults = (testResult: TestResult): Promise<void> => {
     });
 };
 
-const submitTestDone = () => waitForActiveRequestsToBeEmpty().then(() => fetch(`${SERVER_ADDRESS}${Routes.testDone}`, defaultRequestInit));
+const submitTestDone = () => waitForActiveRequestsToBeEmpty().then(() => originalFetch(`${SERVER_ADDRESS}${Routes.testDone}`, defaultRequestInit));
 
 let currentActiveTestConfig: TestConfig | null = null;
 
 const getTestConfig = (): Promise<TestConfig> =>
-    fetch(`${SERVER_ADDRESS}${Routes.testConfig}`, defaultRequestInit)
+    originalFetch(`${SERVER_ADDRESS}${Routes.testConfig}`, defaultRequestInit)
         .then((res: Response): Promise<TestConfig> => res.json())
         .then((config: TestConfig) => {
             currentActiveTestConfig = config;

src/libs/E2E/reactNativeLaunchingTest.ts

@@ -1,3 +1,5 @@
+import Performance, { PERFORMANCE_METRICS } from '@libs/Performance';
+import {setShouldForceOffline} from '@libs/actions/Network';
 /* eslint-disable import/newline-after-import,import/first */
 
 /**
@@ -6,11 +8,10 @@
  * into the actual release app.
  */
 import canCapturePerformanceMetrics from '@libs/Metrics';
-import Performance from '@libs/Performance';
 import Config from 'react-native-config';
 import E2EConfig from '../../../tests/e2e/config';
 import E2EClient from './client';
-import installNetworkInterceptor from './utils/NetworkInterceptor';
+import { disableNetworkRequests } from './utils/NetworkInterceptor';
 import LaunchArgs from './utils/LaunchArgs';
 import type {TestModule, Tests} from './types';
 
@@ -42,20 +43,17 @@
 // Once we receive the TII measurement we know that the app is initialized and ready to be used:
 const appReady = new Promise<void>((resolve) => {
     Performance.subscribeToMeasurements((entry) => {
-        if (entry.name !== 'TTI') {
+        if (entry.name !== PERFORMANCE_METRICS.TTI) {
             return;
         }
 
         resolve();
     });
 });
 
-// Install the network interceptor
-installNetworkInterceptor(
-    () => E2EClient.getNetworkCache(appInstanceId),
-    (networkCache) => E2EClient.updateNetworkCache(appInstanceId, networkCache),
-    LaunchArgs.mockNetwork ?? false
-)
+const disableNetwork = LaunchArgs.mockNetwork ?? false;
+disableNetworkRequests(disableNetwork);
+setShouldForceOffline(disableNetwork);
 
 E2EClient.getTestConfig()
     .then((config): Promise<void> | undefined => {
@@ -75,7 +73,7 @@
         appReady
             .then(() => {
                 console.debug('[E2E] App is ready, running test…');
-                Performance.measureFailSafe('appStartedToReady', 'regularAppStart');
+                Performance.measureFailSafe(PERFORMANCE_METRICS.APP_STARTED_TO_READY, PERFORMANCE_METRICS.REGULAR_APP_START);
                 test(config);
             })
             .catch((error) => {
@@ -87,6 +85,6 @@
     });
 
 // start the usual app
-Performance.markStart('regularAppStart');
-import '../../../index';
-Performance.markEnd('regularAppStart');
+Performance.markStart(PERFORMANCE_METRICS.REGULAR_APP_START);
+import '../../../appIndex';
+Performance.markEnd(PERFORMANCE_METRICS.REGULAR_APP_START);

src/libs/E2E/utils/NetworkInterceptor.ts

@@ -14,6 +14,8 @@ const globalIsNetworkInterceptorInstalledPromise = new Promise<void>((resolve, r
 });
 let networkCache: NetworkCacheMap | null = null;
 
+const originalFetch = global.fetch;
+
 /**
  * The headers of a fetch request can be passed as an array of tuples or as an object.
  * This function converts the headers to an object.
@@ -121,6 +123,14 @@ function waitForActiveRequestsToBeEmpty(): Promise<void> {
     });
 }
 
+function disableNetworkRequests(disable = true) {
+    if (!disable) {
+        return;
+    }
+
+    global.fetch = () => Promise.reject(new Error('Network requests are disabled'));
+}
+
 /**
  * Install a network interceptor by overwriting the global fetch function:
  * - Overwrites fetch globally with a custom implementation
@@ -135,7 +145,6 @@ export default function installNetworkInterceptor(
     shouldReturnRecordedResponse: boolean,
 ) {
     console.debug(LOG_TAG, 'installing with shouldReturnRecordedResponse:', shouldReturnRecordedResponse);
-    const originalFetch = global.fetch;
 
     if (networkCache == null && shouldReturnRecordedResponse) {
         console.debug(LOG_TAG, 'fetching network cache …');
@@ -209,4 +218,5 @@ export default function installNetworkInterceptor(
             });
     };
 }
-export {waitForActiveRequestsToBeEmpty};
+
+export {waitForActiveRequestsToBeEmpty, disableNetworkRequests, originalFetch};

src/libs/Performance.tsx

@@ -10,6 +10,32 @@ import isE2ETestSession from './E2E/isE2ETestSession';
 import getComponentDisplayName from './getComponentDisplayName';
 import canCapturePerformanceMetrics from './Metrics';
 
+// Native performance marks provided by react-native-performance under the 'react-native-mark' type
+const NATIVE_MARKS = {
+    NATIVE_LAUNCH_START: 'nativeLaunchStart',
+    NATIVE_LAUNCH_END: 'nativeLaunchEnd',
+    DOWNLOAD_START: 'downloadStart',
+    DOWNLOAD_END: 'downloadEnd',
+    RUN_JS_BUNDLE_START: 'runJsBundleStart',
+    RUN_JS_BUNDLE_END: 'runJsBundleEnd',
+    APP_CREATION_START: 'appCreationStart',
+    APP_CREATION_END: 'appCreationEnd',
+    CONTENT_APPEARED: 'contentAppeared',
+} as const;
+
+const METRICS = {
+    NATIVE_LAUNCH: 'nativeLaunch',
+    NATIVE_LAUNCH_END_TO_APP_CREATION_START: 'nativeLaunchEnd_To_appCreationStart',
+    APP_CREATION: 'appCreation',
+    APP_CREATION_END_TO_CONTENT_APPEARED: 'appCreationEnd_To_contentAppeared',
+    CONTENT_APPEARED_TO_SCREEN_TTI: 'contentAppeared_To_screenTTI',
+    RUN_JS_BUNDLE: 'runJsBundle',
+    JS_BUNDLE_DOWNLOAD: 'jsBundleDownload',
+    TTI: 'TTI',
+    REGULAR_APP_START: 'regularAppStart',
+    APP_STARTED_TO_READY: 'appStartedToReady',
+} as const;
+
 /**
  * Deep diff between two objects. Useful for figuring out what changed about an object from one render to the next so
  * that state and props updates can be optimized.
@@ -47,7 +73,7 @@ function measureTTI(endMark?: string): void {
     // Make sure TTI is captured when the app is really usable
     InteractionManager.runAfterInteractions(() => {
         requestAnimationFrame(() => {
-            measureFailSafe('TTI', 'nativeLaunchStart', endMark);
+            measureFailSafe(METRICS.TTI, NATIVE_MARKS.NATIVE_LAUNCH_START, endMark);
 
             // We don't want an alert to show:
             // - on builds with performance metrics collection disabled by a feature flag
@@ -66,26 +92,26 @@ function measureTTI(endMark?: string): void {
  */
 const nativeMarksObserver = new PerformanceObserver((list, _observer) => {
     list.getEntries().forEach((entry) => {
-        if (entry.name === 'nativeLaunchEnd') {
-            measureFailSafe('nativeLaunch', 'nativeLaunchStart', 'nativeLaunchEnd');
+        if (entry.name === NATIVE_MARKS.NATIVE_LAUNCH_END) {
+            measureFailSafe(METRICS.NATIVE_LAUNCH, NATIVE_MARKS.NATIVE_LAUNCH_START, NATIVE_MARKS.NATIVE_LAUNCH_END);
         }
-        if (entry.name === 'downloadEnd') {
-            measureFailSafe('jsBundleDownload', 'downloadStart', 'downloadEnd');
+        if (entry.name === NATIVE_MARKS.DOWNLOAD_END) {
+            measureFailSafe(METRICS.JS_BUNDLE_DOWNLOAD, NATIVE_MARKS.DOWNLOAD_START, NATIVE_MARKS.DOWNLOAD_END);
         }
-        if (entry.name === 'runJsBundleEnd') {
-            measureFailSafe('runJsBundle', 'runJsBundleStart', 'runJsBundleEnd');
+        if (entry.name === NATIVE_MARKS.RUN_JS_BUNDLE_END) {
+            measureFailSafe(METRICS.RUN_JS_BUNDLE, NATIVE_MARKS.RUN_JS_BUNDLE_START, NATIVE_MARKS.RUN_JS_BUNDLE_END);
         }
-        if (entry.name === 'appCreationEnd') {
-            measureFailSafe('appCreation', 'appCreationStart', 'appCreationEnd');
-            measureFailSafe('nativeLaunchEnd_To_appCreationStart', 'nativeLaunchEnd', 'appCreationStart');
+        if (entry.name === NATIVE_MARKS.APP_CREATION_END) {
+            measureFailSafe(METRICS.APP_CREATION, NATIVE_MARKS.APP_CREATION_START, NATIVE_MARKS.APP_CREATION_END);
+            measureFailSafe(METRICS.NATIVE_LAUNCH_END_TO_APP_CREATION_START, NATIVE_MARKS.NATIVE_LAUNCH_END, NATIVE_MARKS.APP_CREATION_START);
         }
-        if (entry.name === 'contentAppeared') {
-            measureFailSafe('appCreationEnd_To_contentAppeared', 'appCreationEnd', 'contentAppeared');
+        if (entry.name === NATIVE_MARKS.CONTENT_APPEARED) {
+            measureFailSafe(METRICS.APP_CREATION_END_TO_CONTENT_APPEARED, NATIVE_MARKS.APP_CREATION_END, NATIVE_MARKS.CONTENT_APPEARED);
         }
 
         // At this point we've captured and processed all the native marks we're interested in
         // and are not expecting to have more thus we can safely disconnect the observer
-        if (entry.name === 'runJsBundleEnd' || entry.name === 'downloadEnd') {
+        if (entry.name === NATIVE_MARKS.RUN_JS_BUNDLE_END || entry.name === NATIVE_MARKS.DOWNLOAD_END) {
             _observer.disconnect();
         }
     });
@@ -115,7 +141,7 @@ const customMarksObserver = new PerformanceObserver((list) => {
 
         // Capture any custom measures or metrics below
         if (mark.name === `${CONST.TIMING.SIDEBAR_LOADED}_end`) {
-            measureFailSafe('contentAppeared_To_screenTTI', 'contentAppeared', mark.name);
+            measureFailSafe(METRICS.CONTENT_APPEARED_TO_SCREEN_TTI, NATIVE_MARKS.CONTENT_APPEARED, mark.name);
             measureTTI(mark.name);
         }
     });
@@ -132,18 +158,10 @@ function setCustomMarksObserverEnabled(enabled = false): void {
 }
 
 function getPerformanceMetrics(): PerformanceEntry[] {
-    return [
-        ...performance.getEntriesByName('nativeLaunch'),
-        ...performance.getEntriesByName('nativeLaunchEnd_To_appCreationStart'),
-        ...performance.getEntriesByName('appCreation'),
-        ...performance.getEntriesByName('appCreationEnd_To_contentAppeared'),
-        ...performance.getEntriesByName('contentAppeared_To_screenTTI'),
-        ...performance.getEntriesByName('runJsBundle'),
-        ...performance.getEntriesByName('jsBundleDownload'),
-        ...performance.getEntriesByName('TTI'),
-        ...performance.getEntriesByName('regularAppStart'),
-        ...performance.getEntriesByName('appStartedToReady'),
-    ].filter((entry) => entry.duration > 0);
+    return Object.values(METRICS)
+        .map((name) => performance.getEntriesByName(name))
+        .flat()
+        .filter((entry) => entry.duration > 0);
 }
 
 function getPerformanceMeasures(): PerformanceEntry[] {
@@ -267,3 +285,5 @@ export default {
     markEnd,
     withRenderTrace,
 };
+
+export {METRICS as PERFORMANCE_METRICS, NATIVE_MARKS as PERFORMANCE_MARKS};

tests/e2e/compare/output/markdown.ts

@@ -7,8 +7,15 @@
 import * as format from './format';
 import markdownTable from './markdownTable';
 
-const MAX_CHARACTERS_PER_FILE = 65536;
-const FILE_SIZE_SAFETY_MARGIN = 1000;
+// This is the maximum number of characters allowed to post to a GitHub comment body through the GitHub CLI
+// Total of 65536 4-byte unicode characters, but we're mostly not sending 4 bytes per character
+// const MAX_CHARACTERS_PER_FILE = 65536;
+
+// According to (1.) article and (2.) oist, the maximum number of characters per file is actually 262.144 characters
+// 1. https://github.com/dead-claudia/github-limits (PR Body)
+// 2. https://github.com/orgs/community/discussions/41331#discussioncomment-9276173
+const MAX_CHARACTERS_PER_FILE = 262144;
+const FILE_SIZE_SAFETY_MARGIN = 500;
 const MAX_CHARACTERS_PER_FILE_WITH_SAFETY_MARGIN = MAX_CHARACTERS_PER_FILE - FILE_SIZE_SAFETY_MARGIN;
 
 const tableHeader = ['Name', 'Duration'];
@@ -49,41 +56,49 @@
     return '';
 };
 
-const buildDetailsTable = (entries: Entry[], numberOfTables = 1) => {
-    if (!entries.length) {
-        return [''];
-    }
-
+const splitEntriesPerTable = (entries: Entry[], numberOfTables: number) => {
     // We always need at least one table
     const safeNumberOfTables = numberOfTables === 0 ? 1 : numberOfTables;
 
     const entriesPerTable = Math.floor(entries.length / safeNumberOfTables);
-    const tables: string[] = [];
-    for (let i = 0; i < safeNumberOfTables; i++) {
+    const tables: Entry[][] = [];
+    for (let i = 0; i < numberOfTables; i++) {
         const start = i * entriesPerTable;
-        const end = i === safeNumberOfTables - 1 ? entries.length : start + entriesPerTable;
+        const end = i === numberOfTables - 1 ? entries.length : start + entriesPerTable;
         const tableEntries = entries.slice(start, end);
 
-        const rows = tableEntries.map((entry) => [entry.name, buildDurationDetailsEntry(entry)]);
-        const content = markdownTable([tableHeader, ...rows]);
+        tables.push(tableEntries);
+    }
 
-        const tableMarkdown = collapsibleSection('Show details', content);
+    return tables;
+};
 
-        tables.push(tableMarkdown);
+const buildDetailsTable = (entriesByTable: Entry[][]) => {
+    if (!entriesByTable.length) {
+        return [''];
     }
 
-    return tables;
+    return entriesByTable.map((entries) => {
+        const rows = entries.map((entry) => [entry.name, buildDurationDetailsEntry(entry)]);
+        const content = markdownTable([tableHeader, ...rows]);
+
+        const tableMarkdown = collapsibleSection('Show details', content);
+        return tableMarkdown;
+    });
 };
 
-const buildSummaryTable = (entries: Entry[], collapse = false) => {
-    if (!entries.length) {
-        return '_There are no entries_';
+const buildSummaryTable = (entriesByTable: Entry[][], collapse = false) => {
+    if (!entriesByTable.length) {
+        return ['_There are no entries_'];
     }
 
-    const rows = entries.map((entry) => [entry.name, formatEntryDuration(entry)]);
-    const content = markdownTable([tableHeader, ...rows]);
+    return entriesByTable.map((entries) => {
+        const rows = entries.map((entry) => [entry.name, formatEntryDuration(entry)]);
+        const content = markdownTable([tableHeader, ...rows]);
 
-    return collapse ? collapsibleSection('Show entries', content) : content;
+        const tableMarkdown = collapse ? collapsibleSection('Show entries', content) : content;
+        return tableMarkdown;
+    });
 };
 
 const buildMarkdown = (data: Data, skippedTests: string[], numberOfExtraFiles?: number): [string, ...string[]] => {
@@ -126,16 +141,18 @@
     }
 
     mainFile += '\n\n### Significant Changes To Duration';
-    mainFile += `\n${buildSummaryTable(data.significance)}`;
-    mainFile += `\n${buildDetailsTable(data.significance, 1).at(0)}`;
+    mainFile += `\n\n${buildSummaryTable([data.significance]).at(0)}`;
+    mainFile += `\n${buildDetailsTable([data.significance]).at(0)}`;
 
     // We always need at least one table
-    const numberOfMeaninglessDetailsTables = nExtraFiles === 0 ? 1 : nExtraFiles;
-    const meaninglessDetailsTables = buildDetailsTable(data.meaningless, numberOfMeaninglessDetailsTables);
+    const numberOfMeaninglessDataTables = nExtraFiles === 0 ? 1 : nExtraFiles;
+    const meaninglessEntriesPerTable = splitEntriesPerTable(data.meaningless, numberOfMeaninglessDataTables);
+    const meaninglessSummaryTable = buildSummaryTable(meaninglessEntriesPerTable, true);
+    const meaninglessDetailsTables = buildDetailsTable(meaninglessEntriesPerTable);
 
     if (nExtraFiles === 0) {
         mainFile += '\n\n### Meaningless Changes To Duration';
-        mainFile += `\n${buildSummaryTable(data.meaningless, true)}`;
+        mainFile += `\n\n${meaninglessSummaryTable.at(0)}`;
         mainFile += `\n${meaninglessDetailsTables.at(0)}`;
 
         return [mainFile];
@@ -147,9 +164,12 @@
         extraFile += ` (${i + 2}/${nExtraFiles + 1})`;
 
         extraFile += '\n\n### Meaningless Changes To Duration';
-        extraFile += nExtraFiles >= 2 ? ` (${i + 1}/${nExtraFiles})` : '';
 
-        extraFile += `\n${buildSummaryTable(data.meaningless, true)}`;
+        const entries = meaninglessEntriesPerTable.at(i);
+        const fromToString = `\n> ${entries?.at(0)?.name} - ${entries?.at(-1)?.name}`;
+        extraFile += nExtraFiles >= 2 ? fromToString : '';
+
+        extraFile += `\n\n${meaninglessSummaryTable.at(i)}`;
         extraFile += `\n${meaninglessDetailsTables.at(i)}`;
         extraFile += '\n';