From e9ec9fbde480655b2c5a5a9b89e33485fb1335ca Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Wed, 25 Sep 2024 19:28:05 -0700 Subject: [PATCH] Refactor LogBox tests to spies (#46638) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46638 This is annoying, but in the next diff that fixes a bug I need to test using the default warning filter instead of a mock (really, all this mocking is terrible, idk why I did it this way). Unfortunately, in Jest you can't just reset mocks from `jest.mock`, `restoreMocks` only resets spies and not mocks (wild right). So in this diff I converted all the `jest.mock` calls to `jest.spyOn`. I also corrected some of the mocks that require `monitorEvent: 'warning',` like the warning filter sets. I also added a test that works without the fix. Changelog: [Internal] Reviewed By: javache Differential Revision: D63349615 fbshipit-source-id: 4f2a5a8800c8fe1a10e3613d3c2d0ed02fca773e --- .../Libraries/LogBox/__tests__/LogBox-test.js | 121 +++++++++++++----- 1 file changed, 91 insertions(+), 30 deletions(-) diff --git a/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js b/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js index be5ea785bbb..b44c6131ffb 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js +++ b/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js @@ -13,6 +13,7 @@ const LogBoxData = require('../Data/LogBoxData'); const LogBox = require('../LogBox').default; +const ExceptionsManager = require('../../Core/ExceptionsManager.js'); declare var console: any; @@ -34,15 +35,18 @@ describe('LogBox', () => { beforeEach(() => { jest.resetModules(); + jest.restoreAllMocks(); console.error = jest.fn(); - console.log = jest.fn(); console.warn = jest.fn(); }); afterEach(() => { LogBox.uninstall(); + // Reset ExceptionManager patching. + if (console._errorOriginal) { + console._errorOriginal = null; + } console.error = error; - console.log = log; console.warn = warn; }); @@ -95,7 +99,7 @@ describe('LogBox', () => { }); it('registers warnings', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -105,13 +109,14 @@ describe('LogBox', () => { }); it('reports a LogBox exception if we fail to add warnings', () => { - jest.mock('../Data/LogBoxData'); - const mockError = new Error('Simulated error'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'reportLogBoxError'); // Picking a random implementation detail to simulate throwing. - (LogBoxData.isMessageIgnored: any).mockImplementation(() => { + jest.spyOn(LogBoxData, 'isMessageIgnored').mockImplementation(() => { throw mockError; }); + const mockError = new Error('Simulated error'); LogBox.install(); @@ -123,7 +128,8 @@ describe('LogBox', () => { }); it('only registers errors beginning with "Warning: "', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); LogBox.install(); @@ -133,7 +139,8 @@ describe('LogBox', () => { }); it('registers react errors with the formatting from filter', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ finalFormat: 'Custom format', @@ -157,7 +164,8 @@ describe('LogBox', () => { }); it('registers errors with component stack as errors by default', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({}); @@ -174,7 +182,8 @@ describe('LogBox', () => { }); it('registers errors with component stack as errors by default if not found in warning filter', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ monitorEvent: 'warning_unhandled', @@ -193,10 +202,12 @@ describe('LogBox', () => { }); it('registers errors with component stack with legacy suppression as warning', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ suppressDialog_LEGACY: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -211,10 +222,12 @@ describe('LogBox', () => { }); it('registers errors with component stack and a forced dialog as fatals', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ forceDialogImmediately: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -229,7 +242,8 @@ describe('LogBox', () => { }); it('registers warning module errors with the formatting from filter', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ finalFormat: 'Custom format', @@ -248,7 +262,8 @@ describe('LogBox', () => { }); it('registers warning module errors as errors by default', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({}); @@ -262,10 +277,12 @@ describe('LogBox', () => { }); it('registers warning module errors with only legacy suppression as warning', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ suppressDialog_LEGACY: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -277,10 +294,12 @@ describe('LogBox', () => { }); it('registers warning module errors with a forced dialog as fatals', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ forceDialogImmediately: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -292,10 +311,12 @@ describe('LogBox', () => { }); it('ignores warning module errors that are suppressed completely', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ suppressCompletely: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -305,10 +326,11 @@ describe('LogBox', () => { }); it('ignores warning module errors that are pattern ignored', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'isMessageIgnored').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); mockFilterResult({}); - (LogBoxData.isMessageIgnored: any).mockReturnValue(true); LogBox.install(); @@ -317,10 +339,11 @@ describe('LogBox', () => { }); it('ignores warning module errors that are from LogBox itself', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'isLogBoxErrorMessage').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); mockFilterResult({}); - (LogBoxData.isLogBoxErrorMessage: any).mockReturnValue(true); LogBox.install(); @@ -329,8 +352,9 @@ describe('LogBox', () => { }); it('ignores logs that are pattern ignored"', () => { - jest.mock('../Data/LogBoxData'); - (LogBoxData.isMessageIgnored: any).mockReturnValue(true); + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'isMessageIgnored').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -339,8 +363,8 @@ describe('LogBox', () => { }); it('does not add logs that are from LogBox itself"', () => { - jest.mock('../Data/LogBoxData'); - (LogBoxData.isLogBoxErrorMessage: any).mockReturnValue(true); + jest.spyOn(LogBoxData, 'isLogBoxErrorMessage').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -349,7 +373,7 @@ describe('LogBox', () => { }); it('ignores logs starting with "(ADVICE)"', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -358,7 +382,7 @@ describe('LogBox', () => { }); it('does not ignore logs formatted to start with "(ADVICE)"', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -376,7 +400,7 @@ describe('LogBox', () => { }); it('ignores console methods after uninstalling', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); LogBox.uninstall(); @@ -389,7 +413,7 @@ describe('LogBox', () => { }); it('does not add logs after uninstalling', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); LogBox.uninstall(); @@ -406,7 +430,7 @@ describe('LogBox', () => { }); it('does not add exceptions after uninstalling', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addException'); LogBox.install(); LogBox.uninstall(); @@ -482,4 +506,41 @@ describe('LogBox', () => { 'Custom: after installing for the second time', ); }); + it('registers errors without component stack as errors by default, when ExceptionManager is registered first', () => { + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'addException'); + + ExceptionsManager.installConsoleErrorReporter(); + LogBox.install(); + + console.error('HIT'); + + // Errors without a component stack skip the warning filter and + // fall through to the ExceptionManager, which are then reported + // back to LogBox as non-fatal exceptions, in a convuluted dance + // in the most legacy cruft way. + expect(LogBoxData.addException).toBeCalledWith( + expect.objectContaining({originalMessage: 'HIT'}), + ); + expect(LogBoxData.checkWarningFilter).not.toBeCalled(); + }); + + it('registers errors without component stack as errors by default, when ExceptionManager is registered second', () => { + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'addException'); + + LogBox.install(); + ExceptionsManager.installConsoleErrorReporter(); + + console.error('HIT'); + + // Errors without a component stack skip the warning filter and + // fall through to the ExceptionManager, which are then reported + // back to LogBox as non-fatal exceptions, in a convuluted dance + // in the most legacy cruft way. + expect(LogBoxData.addException).toBeCalledWith( + expect.objectContaining({originalMessage: 'HIT'}), + ); + expect(LogBoxData.checkWarningFilter).not.toBeCalled(); + }); });