From d43972e5f405876c992b83a92b1f96d190a5e39a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 24 Jun 2025 14:29:17 +0100 Subject: [PATCH] Reduce flakiness with TestSequencer Test change 1 Test with clearInternals Skipping OnyxUtils Test change 2 Test change 3 Cleanup Skip useOnyx instead Revert skip Upload Reassure results Include hidden files in upload step Test change Test change 2 Use ubuntu-24.04-v4 image Test change 3 Do all Reassure steps in one workflow step Test change 4 Test change 5 Test change 6 Test change 7 Reintroduce clearOnyxUtilsInternals Minor adjustments Test change Add test sequencer Cleanup Cleanup --- .github/workflows/reassurePerfTests.yml | 14 ++++++++++--- jest-sequencer.js | 21 +++++++++++++++++++ jest.config.js | 1 + lib/OnyxUtils.ts | 23 +++++++++++++++----- tests/perf-test/OnyxUtils.perf-test.ts | 28 +++++++++++++------------ 5 files changed, 66 insertions(+), 21 deletions(-) create mode 100644 jest-sequencer.js diff --git a/.github/workflows/reassurePerfTests.yml b/.github/workflows/reassurePerfTests.yml index c842494e8..946803fca 100644 --- a/.github/workflows/reassurePerfTests.yml +++ b/.github/workflows/reassurePerfTests.yml @@ -10,7 +10,7 @@ jobs: # Note: We run baseline and delta performance checks in the same runner to reduce hardware variance across machines perf-tests: if: ${{ github.actor != 'OSBotify' }} - runs-on: ubuntu-latest + runs-on: ubuntu-24.04-v4 steps: # v4 - name: Checkout @@ -28,7 +28,7 @@ jobs: run: npm install - name: Run Reassure baseline tests - run: npx reassure --baseline + run: npx reassure --baseline --verbose - name: Checkout PR head SHA run: | @@ -39,7 +39,15 @@ jobs: run: npm install --force - name: Run Reassure delta tests - run: npx reassure --branch + run: npx reassure --branch --verbose + + - name: Upload Reassure results + uses: actions/upload-artifact@v4 + with: + name: results + path: .reassure/ + if-no-files-found: ignore + include-hidden-files: true - name: Validate output.json id: validateReassureOutput diff --git a/jest-sequencer.js b/jest-sequencer.js new file mode 100644 index 000000000..7d68ba4cc --- /dev/null +++ b/jest-sequencer.js @@ -0,0 +1,21 @@ +const Sequencer = require('@jest/test-sequencer').default; + +/** + * Makes all unit tests run ordered by their path, reducing flakiness on Reassure. + */ +class TestSequencer extends Sequencer { + shard(tests, {shardIndex, shardCount}) { + const shardSize = Math.ceil(tests.length / shardCount); + const shardStart = shardSize * (shardIndex - 1); + const shardEnd = shardSize * shardIndex; + + return [...tests].sort((a, b) => (a.path > b.path ? 1 : -1)).slice(shardStart, shardEnd); + } + + sort(tests) { + const copyTests = [...tests]; + return copyTests.sort((testA, testB) => (testA.path > testB.path ? 1 : -1)); + } +} + +module.exports = TestSequencer; diff --git a/jest.config.js b/jest.config.js index 4fc0584dd..b6f148ece 100644 --- a/jest.config.js +++ b/jest.config.js @@ -14,4 +14,5 @@ module.exports = { setupFilesAfterEnv: ['./jestSetup.js'], testTimeout: 60000, transformIgnorePatterns: ['node_modules/(?!((@)?react-native|@ngneat/falso|uuid)/)'], + testSequencer: './jest-sequencer.js', }; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 2f5789991..0e4dd0895 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -49,17 +49,17 @@ const METHOD = { type OnyxMethod = ValueOf; // Key/value store of Onyx key and arrays of values to merge -const mergeQueue: Record>> = {}; -const mergeQueuePromise: Record> = {}; +let mergeQueue: Record>> = {}; +let mergeQueuePromise: Record> = {}; // Holds a mapping of all the React components that want their state subscribed to a store key -const callbackToStateMapping: Record> = {}; +let callbackToStateMapping: Record> = {}; // Keeps a copy of the values of the onyx collection keys as a map for faster lookups let onyxCollectionKeySet = new Set(); // Holds a mapping of the connected key to the subscriptionID for faster lookups -const onyxKeyToSubscriptionIDs = new Map(); +let onyxKeyToSubscriptionIDs = new Map(); // Optional user-provided key value states set when Onyx initializes or clears let defaultKeyStates: Record> = {}; @@ -68,7 +68,7 @@ let batchUpdatesPromise: Promise | null = null; let batchUpdatesQueue: Array<() => void> = []; // Used for comparison with a new update to avoid invoking the Onyx.connect callback with the same data. -const lastConnectionCallbackData = new Map>(); +let lastConnectionCallbackData = new Map>(); let snapshotKey: OnyxKey | null = null; @@ -1450,6 +1450,18 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array< return promises; } +/** + * Clear internal variables used in this file, useful in test environments. + */ +function clearOnyxUtilsInternals() { + mergeQueue = {}; + mergeQueuePromise = {}; + callbackToStateMapping = {}; + onyxKeyToSubscriptionIDs = new Map(); + batchUpdatesQueue = []; + lastConnectionCallbackData = new Map(); +} + const OnyxUtils = { METHOD, getMergeQueue, @@ -1551,3 +1563,4 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { export type {OnyxMethod}; export default OnyxUtils; +export {clearOnyxUtilsInternals}; diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index e187a4d3e..113d6f481 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -4,7 +4,7 @@ import type {OnyxKey, Selector} from '../../lib'; import Onyx from '../../lib'; import StorageMock from '../../lib/storage'; import OnyxCache from '../../lib/OnyxCache'; -import OnyxUtils from '../../lib/OnyxUtils'; +import OnyxUtils, {clearOnyxUtilsInternals} from '../../lib/OnyxUtils'; import type GenericCollection from '../utils/GenericCollection'; import type {Mapping, OnyxUpdate} from '../../lib/Onyx'; import createDeferredTask from '../../lib/createDeferredTask'; @@ -44,6 +44,7 @@ const mockedReportActionsMap = getRandomReportActions(collectionKey); const mockedReportActionsKeys = Object.keys(mockedReportActionsMap); const clearOnyxAfterEachMeasure = async () => { + clearOnyxUtilsInternals(); await Onyx.clear(); }; @@ -59,6 +60,7 @@ describe('OnyxUtils', () => { }); afterEach(async () => { + clearOnyxUtilsInternals(); await Onyx.clear(); }); @@ -321,7 +323,6 @@ describe('OnyxUtils', () => { }); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); mockedReportActionsKeys.forEach((key) => { const id = subscriptionMap.get(key); if (id) { @@ -329,6 +330,7 @@ describe('OnyxUtils', () => { } }); subscriptionMap.clear(); + await clearOnyxAfterEachMeasure(); }, }); }); @@ -351,11 +353,11 @@ describe('OnyxUtils', () => { } }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); subscriptionIDs.forEach((id) => { OnyxUtils.unsubscribeFromKey(id); }); subscriptionIDs.clear(); + await clearOnyxAfterEachMeasure(); }, }); }); @@ -384,10 +386,10 @@ describe('OnyxUtils', () => { subscriptionID = OnyxUtils.subscribeToKey({key: collectionKey, callback: jest.fn(), initWithStoredValues: false}); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); if (subscriptionID) { OnyxUtils.unsubscribeFromKey(subscriptionID); } + await clearOnyxAfterEachMeasure(); }, }, ); @@ -417,10 +419,10 @@ describe('OnyxUtils', () => { subscriptionID = OnyxUtils.subscribeToKey({key: collectionKey, callback: jest.fn(), initWithStoredValues: false}); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); if (subscriptionID) { OnyxUtils.unsubscribeFromKey(subscriptionID); } + await clearOnyxAfterEachMeasure(); }, }, ); @@ -466,10 +468,10 @@ describe('OnyxUtils', () => { await Onyx.multiSet(mockedReportActionsMap); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); if (subscriptionID) { OnyxUtils.unsubscribeFromKey(subscriptionID); } + await clearOnyxAfterEachMeasure(); }, }, ); @@ -495,7 +497,6 @@ describe('OnyxUtils', () => { }); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); mockedReportActionsKeys.forEach((key) => { const id = subscriptionMap.get(key); if (id) { @@ -503,6 +504,7 @@ describe('OnyxUtils', () => { } }); subscriptionMap.clear(); + await clearOnyxAfterEachMeasure(); }, }, ); @@ -526,7 +528,6 @@ describe('OnyxUtils', () => { }); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); mockedReportActionsKeys.forEach((key) => { const id = subscriptionMap.get(key); if (id) { @@ -534,6 +535,7 @@ describe('OnyxUtils', () => { } }); subscriptionMap.clear(); + await clearOnyxAfterEachMeasure(); }, }); }); @@ -726,8 +728,8 @@ describe('OnyxUtils', () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); OnyxUtils.unsubscribeFromKey(subscriptionID); + await clearOnyxAfterEachMeasure(); }, }, ); @@ -753,8 +755,8 @@ describe('OnyxUtils', () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); OnyxUtils.unsubscribeFromKey(subscriptionID); + await clearOnyxAfterEachMeasure(); }, }, ); @@ -822,9 +824,9 @@ describe('OnyxUtils', () => { }); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); OnyxUtils.deleteKeyBySubscriptions(subscriptionID); OnyxUtils.unsubscribeFromKey(subscriptionID); + await clearOnyxAfterEachMeasure(); }, }); }); @@ -844,8 +846,8 @@ describe('OnyxUtils', () => { OnyxUtils.storeKeyBySubscriptions(key, subscriptionID); }, afterEach: async () => { - await clearOnyxAfterEachMeasure(); OnyxUtils.unsubscribeFromKey(subscriptionID); + await clearOnyxAfterEachMeasure(); }, }); }); @@ -865,8 +867,8 @@ describe('OnyxUtils', () => { }), { afterEach: async () => { - await clearOnyxAfterEachMeasure(); OnyxCache.removeLastAccessedKey(key); + await clearOnyxAfterEachMeasure(); }, }, );