From 320398afe560d5591ee5097171acf92f3e48874c Mon Sep 17 00:00:00 2001 From: makitake2013 Date: Sun, 12 Jul 2020 00:27:13 -0700 Subject: [PATCH] Test fixes related to changes in Jest's Fake Timer behavior (#29011) Summary: Fixed tests related to changing Jest's fake timer behavior. And some of the test code in LogBoxData-test.js itself was incorrect so I am fixing it. LogBoxData-test.js fails when I try to build a new local development environment and fork the React Native source to run a JavaScript test. The error message is: runAllImmediates is not available when using modern timers After checking, runAllImmediates could not be used with the following modification. https://github.com/facebook/jest/commit/71631f6bf9ccf8937f02a5ecfb64b62712b86c19 ## Changelog [Internal] [Fixed] - Test fixes related to changes in Jest's Fake Timer behavior Pull Request resolved: https://github.com/facebook/react-native/pull/29011 Test Plan: Ran JavaScript Tests locally. Reviewed By: cpojer Differential Revision: D22494991 Pulled By: rickhanlonii fbshipit-source-id: 4deeaf82b5092ff8b60c4606eb45549beb452a5f --- .../LogBox/Data/__tests__/LogBoxData-test.js | 180 +++++++++--------- 1 file changed, 85 insertions(+), 95 deletions(-) diff --git a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index b0dfc3074aa..2c0df65b9aa 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -51,7 +51,7 @@ const observe = () => { }; }; -const addLogs = logs => { +const addLogs = (logs, options) => { logs.forEach(message => { LogBoxData.addLog({ level: 'warn', @@ -62,10 +62,13 @@ const addLogs = logs => { category: message, componentStack: [], }); + if (options == null || options.flush !== false) { + jest.runOnlyPendingTimers(); + } }); }; -const addSoftErrors = errors => { +const addSoftErrors = (errors, options) => { errors.forEach(error => { LogBoxData.addException( Object.assign( @@ -85,10 +88,13 @@ const addSoftErrors = errors => { : error, ), ); + if (options == null || options.flush !== false) { + jest.runOnlyPendingTimers(); + } }); }; -const addFatalErrors = errors => { +const addFatalErrors = (errors, options) => { errors.forEach(error => { LogBoxData.addException( Object.assign( @@ -108,37 +114,43 @@ const addFatalErrors = errors => { : error, ), ); + if (options == null || options.flush !== false) { + // Errors include two timers, the second is for optimistic symbolication. + jest.runOnlyPendingTimers(); + jest.runOnlyPendingTimers(); + } }); }; -const addSyntaxError = () => { - addFatalErrors([ - { - message: ` - +const addSyntaxError = options => { + addFatalErrors( + [ + { + message: ` 197 | }); 198 | > 199 | export default CrashReactApp; | ^ 200 |`, - originalMessage: `TransformError SyntaxError: /path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js: 'import' and 'export' may only appear at the top level (199:0) - + originalMessage: `TransformError SyntaxError: /path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js: 'import' and 'export' may only appear at the top level (199:0) 197 | }); 198 | > 199 | export default CrashReactApp; | ^ 200 |`, - }, - ]); + }, + ], + options, + ); }; beforeEach(() => { jest.resetModules(); }); -const flushLogs = () => { - jest.runAllImmediates(); - jest.runAllTimers(); +const flushToObservers = () => { + // Observer updates are debounced and need to advance timers to flush. + jest.runOnlyPendingTimers(); }; describe('LogBoxData', () => { @@ -147,7 +159,6 @@ describe('LogBoxData', () => { addSoftErrors(['B']); addFatalErrors(['C']); addSyntaxError(); - flushLogs(); expect(registry().length).toBe(4); expect(registry()[0]).toBeDefined(); @@ -169,10 +180,9 @@ describe('LogBoxData', () => { addSoftErrors(['C', 'D']); addFatalErrors(['E', 'F']); addSyntaxError(); - flushLogs(); expect(registry().length).toBe(7); - expect(selectedLogIndex()).toBe(4); // Fatal index. + expect(selectedLogIndex()).toBe(6); // Syntax error index. LogBoxData.clear(); expect(registry().length).toBe(0); @@ -183,14 +193,13 @@ describe('LogBoxData', () => { addLogs(['A', 'B']); addSoftErrors(['C', 'D', 'E']); addSyntaxError(); - flushLogs(); expect(registry().length).toBe(6); - expect(selectedLogIndex()).toBe(5); // Fatal index. + expect(selectedLogIndex()).toBe(5); // Syntax error index. LogBoxData.clearWarnings(); expect(registry().length).toBe(4); - expect(selectedLogIndex()).toBe(3); // New fatal index. + expect(selectedLogIndex()).toBe(3); // New syntax error index. }); it('clears errors and fatals, but not syntax errors.', () => { @@ -198,14 +207,13 @@ describe('LogBoxData', () => { addSoftErrors(['C', 'D', 'E']); addFatalErrors(['F']); addSyntaxError(); - flushLogs(); expect(registry().length).toBe(7); - expect(selectedLogIndex()).toBe(5); // Fatal index. + expect(selectedLogIndex()).toBe(6); // Syntax error index. LogBoxData.clearErrors(); expect(registry().length).toBe(3); - expect(selectedLogIndex()).toBe(2); // New Fatal index. + expect(selectedLogIndex()).toBe(2); // New syntax error index. }); it('clears all types except syntax errors', () => { @@ -213,24 +221,25 @@ describe('LogBoxData', () => { addSoftErrors(['C', 'D', 'E']); addFatalErrors(['F']); addSyntaxError(); - flushLogs(); expect(registry().length).toBe(7); - expect(selectedLogIndex()).toBe(5); // Fatal index. + expect(selectedLogIndex()).toBe(6); // Syntax error index. LogBoxData.clearErrors(); LogBoxData.clearWarnings(); expect(registry().length).toBe(1); - expect(selectedLogIndex()).toBe(0); // New Fatal index. + expect(selectedLogIndex()).toBe(0); // New syntax error index. }); it('keeps logs in chronological order', () => { - addLogs(['A']); - addSoftErrors(['B']); - addFatalErrors(['C']); - flushLogs(); + addLogs(['A'], {flush: false}); + addSoftErrors(['B'], {flush: false}); + addFatalErrors(['C'], {flush: false}); + + // Flush logs manually. + jest.runAllTimers(); + addLogs(['D']); - flushLogs(); let logs = registry(); expect(logs.length).toBe(4); @@ -240,7 +249,6 @@ describe('LogBoxData', () => { expect(logs[3].category).toEqual('D'); addLogs(['A']); - jest.runAllImmediates(); // Expect `A` to be added to the end of the registry. logs = registry(); @@ -256,40 +264,39 @@ describe('LogBoxData', () => { expect(selectedLogIndex()).toBe(-1); LogBoxData.setSelectedLog(1); - flushLogs(); expect(selectedLogIndex()).toBe(1); }); it('does not set the selectedLogIndex for warnings', () => { addLogs(['A']); - flushLogs(); expect(selectedLogIndex()).toBe(-1); }); it('does not set the selectedLogIndex for soft errors', () => { addSoftErrors(['A']); - flushLogs(); expect(selectedLogIndex()).toBe(-1); }); it('sets the selectedLogIndex to the first fatal error (after symbolication)', () => { - addFatalErrors(['A']); + addFatalErrors(['A'], {flush: false}); - // Order maters for symbolication before timeout. - flushLogs(); - jest.runAllTimers(); + // Order maters for testing symbolication before timeout. + jest.runOnlyPendingTimers(); // Trigger appendNewLog. + jest.advanceTimersByTime(10); // Trigger symbolication but not timeout. + jest.runAllTimers(); // Flush remaining. expect(selectedLogIndex()).toBe(0); - addLogs(['B']); - addFatalErrors(['C']); + addLogs(['B'], {flush: false}); + addFatalErrors(['C'], {flush: false}); - // Order maters for symbolication before timeout. - flushLogs(); - jest.runAllTimers(); + // Order maters for testing symbolication before timeout. + jest.runOnlyPendingTimers(); // Trigger appendNewLogs. + jest.advanceTimersByTime(10); // Trigger symbolication. + jest.runAllTimers(); // Flush remaining. // This should still be 0 (the first fatal exception) // becuase it is the most likely source of the error. @@ -299,20 +306,22 @@ describe('LogBoxData', () => { }); it('sets the selectedLogIndex to the first fatal error (hitting timeout limit)', () => { - addFatalErrors(['A']); + addFatalErrors(['A'], {flush: false}); - // Order maters for timeout before symbolication. - jest.runAllTimers(); - flushLogs(); + // Order maters for testing timeout before symbolication. + jest.runOnlyPendingTimers(); // Trigger appendNewLog. + jest.advanceTimersByTime(1001); // Trigger OPTIMISTIC_WAIT_TIME timeout. + jest.runAllTimers(); // Flush remaining. expect(selectedLogIndex()).toBe(0); addLogs(['B']); addFatalErrors(['C']); - // Order maters for timeout before symbolication. - jest.runAllTimers(); - flushLogs(); + // Order maters for testing timeout before symbolication. + jest.runOnlyPendingTimers(); // Trigger appendNewLogs. + jest.advanceTimersByTime(1001); // Trigger OPTIMISTIC_WAIT_TIME timeout. + jest.runAllTimers(); // Flush remaining. // This should still be 0 (the first fatal exception) // becuase it is the most likely source of the error. @@ -323,45 +332,38 @@ describe('LogBoxData', () => { it('sets the selectedLogIndex to the last syntax error', () => { addSyntaxError(); - flushLogs(); expect(selectedLogIndex()).toBe(0); addLogs(['B']); addSyntaxError(); - flushLogs(); expect(selectedLogIndex()).toBe(2); }); it('keeps selectedLogIndex set to the syntax error even when a new fatal is added', () => { addSyntaxError(); - flushLogs(); expect(selectedLogIndex()).toBe(0); addLogs(['B']); addFatalErrors(['C']); - flushLogs(); expect(selectedLogIndex()).toBe(0); }); it('keeps selectedLogIndex set to the syntax error even when explicitly changed', () => { addSyntaxError(); - flushLogs(); expect(selectedLogIndex()).toBe(0); LogBoxData.setSelectedLog(1); - flushLogs(); expect(selectedLogIndex()).toBe(0); }); it('increments the count of previous log with matching category (logs)', () => { addLogs(['A', 'B']); - flushLogs(); let logs = registry(); expect(logs.length).toBe(2); @@ -371,7 +373,6 @@ describe('LogBoxData', () => { expect(logs[1].count).toBe(1); addLogs(['B']); - flushLogs(); // Expect `B` to be rolled into the last log. logs = registry(); @@ -384,7 +385,6 @@ describe('LogBoxData', () => { it('increments the count of previous log with matching category (errors)', () => { addFatalErrors(['A', 'B']); - flushLogs(); let logs = registry(); expect(logs.length).toBe(2); @@ -394,7 +394,6 @@ describe('LogBoxData', () => { expect(logs[1].count).toBe(1); addSoftErrors(['B']); - flushLogs(); // Expect `B` to be rolled into the last log. logs = registry(); @@ -407,14 +406,12 @@ describe('LogBoxData', () => { it('increments the count of previous log with matching category (syntax)', () => { addSyntaxError(); - flushLogs(); let logs = registry(); expect(logs.length).toBe(1); expect(logs[0].count).toBe(1); addSyntaxError(); - flushLogs(); logs = registry(); expect(logs.length).toBe(1); @@ -423,7 +420,6 @@ describe('LogBoxData', () => { it('ignores logs matching patterns (logs)', () => { addLogs(['A!', 'B?', 'C!']); - flushLogs(); expect(filteredRegistry().length).toBe(3); @@ -437,7 +433,6 @@ describe('LogBoxData', () => { it('ignores logs matching patterns (errors)', () => { addSoftErrors(['A!', 'B?']); addFatalErrors(['C!']); - flushLogs(); expect(filteredRegistry().length).toBe(3); @@ -450,7 +445,6 @@ describe('LogBoxData', () => { it('ignores matching regexs or pattern (logs)', () => { addLogs(['There are 4 dogs', 'There are 3 cats', 'There are H cats']); - flushLogs(); expect(filteredRegistry().length).toBe(3); @@ -467,7 +461,6 @@ describe('LogBoxData', () => { it('ignores matching regexs or pattern (errors)', () => { addSoftErrors(['There are 4 dogs', 'There are 3 cats']); addFatalErrors(['There are H cats']); - flushLogs(); expect(filteredRegistry().length).toBe(3); @@ -486,7 +479,6 @@ describe('LogBoxData', () => { addSoftErrors(['B?']); addFatalErrors(['C!']); addSyntaxError(); - flushLogs(); expect(registry().length).toBe(4); expect(disabledState()).toBe(false); @@ -516,7 +508,7 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); addLogs(['A']); - flushLogs(); + flushToObservers(); expect(observer.mock.calls.length).toBe(2); // We expect observers to recieve the same Set object in sequential updates @@ -541,17 +533,17 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); addLogs(['A']); - flushLogs(); + flushToObservers(); expect(observer.mock.calls.length).toBe(2); const lastLog = Array.from(observer.mock.calls[1][0].logs)[0]; LogBoxData.dismiss(lastLog); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); // Does nothing when category does not exist. LogBoxData.dismiss(lastLog); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); }); @@ -560,16 +552,16 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); addLogs(['A']); - flushLogs(); + flushToObservers(); expect(observer.mock.calls.length).toBe(2); LogBoxData.clear(); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); // Does nothing when already empty. LogBoxData.clear(); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); }); @@ -581,17 +573,16 @@ describe('LogBoxData', () => { addSoftErrors(['B']); addFatalErrors(['C']); addSyntaxError(); - jest.runAllImmediates(); - expect(observer.mock.calls.length).toBe(2); + expect(observer.mock.calls.length).toBe(5); LogBoxData.clearWarnings(); - jest.runAllImmediates(); - expect(observer.mock.calls.length).toBe(3); + flushToObservers(); + expect(observer.mock.calls.length).toBe(6); // Does nothing when already empty. LogBoxData.clearWarnings(); - jest.runAllImmediates(); - expect(observer.mock.calls.length).toBe(3); + flushToObservers(); + expect(observer.mock.calls.length).toBe(6); }); it('updates observers when errors cleared', () => { @@ -602,17 +593,16 @@ describe('LogBoxData', () => { addSoftErrors(['B']); addFatalErrors(['C']); addSyntaxError(); - jest.runAllImmediates(); - expect(observer.mock.calls.length).toBe(2); + expect(observer.mock.calls.length).toBe(5); LogBoxData.clearErrors(); - jest.runAllImmediates(); - expect(observer.mock.calls.length).toBe(3); + flushToObservers(); + expect(observer.mock.calls.length).toBe(6); // Does nothing when already empty. LogBoxData.clearErrors(); - jest.runAllImmediates(); - expect(observer.mock.calls.length).toBe(3); + flushToObservers(); + expect(observer.mock.calls.length).toBe(6); }); it('updates observers when an ignore pattern is added', () => { @@ -620,16 +610,16 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); LogBoxData.addIgnorePatterns(['?']); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(2); LogBoxData.addIgnorePatterns(['!']); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); // Does nothing for an existing ignore pattern. LogBoxData.addIgnorePatterns(['!']); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); }); @@ -638,21 +628,21 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); LogBoxData.setDisabled(true); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(2); // Does nothing when already disabled. LogBoxData.setDisabled(true); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(2); LogBoxData.setDisabled(false); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); // Does nothing when already enabled. LogBoxData.setDisabled(false); - jest.runAllImmediates(); + flushToObservers(); expect(observer.mock.calls.length).toBe(3); });