Skip to content

Commit

Permalink
Refactor LogBox tests to spies (#46638)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #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
  • Loading branch information
@rickhanlonii @facebook-github-bot
rickhanlonii authored and facebook-github-bot committed Sep 25, 2024
1 parent ec53ba3 commit 4d09c12
Showing 1 changed file with 91 additions and 30 deletions.
121 changes: 91 additions & 30 deletions packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

const LogBoxData = require('../Data/LogBoxData');
const LogBox = require('../LogBox').default;
const ExceptionsManager = require('../../Core/ExceptionsManager.js');

declare var console: any;

Expand All @@ -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;
});

Expand Down Expand Up @@ -95,7 +99,7 @@ describe('LogBox', () => {
});

it('registers warnings', () => {
jest.mock('../Data/LogBoxData');
jest.spyOn(LogBoxData, 'addLog');

LogBox.install();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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',
Expand All @@ -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({});

Expand All @@ -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',
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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',
Expand All @@ -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({});

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -349,7 +373,7 @@ describe('LogBox', () => {
});

it('ignores logs starting with "(ADVICE)"', () => {
jest.mock('../Data/LogBoxData');
jest.spyOn(LogBoxData, 'addLog');

LogBox.install();

Expand All @@ -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();

Expand All @@ -376,7 +400,7 @@ describe('LogBox', () => {
});

it('ignores console methods after uninstalling', () => {
jest.mock('../Data/LogBoxData');
jest.spyOn(LogBoxData, 'addLog');

LogBox.install();
LogBox.uninstall();
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
});
});

0 comments on commit 4d09c12

Please sign in to comment.