Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor LogBox tests to spies #46638

Closed
wants to merge 3 commits into from
Closed

Refactor LogBox tests to spies #46638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f273cb
Re-enable integration tests
rickhanlonii Sep 25, 2024
02489d0
Add integration tests for console errors + ExceptionManager
rickhanlonii Sep 25, 2024
eaf652e
Refactor LogBox tests to spies (#46638)
rickhanlonii Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 169 additions & 44 deletions packages/react-native/Libraries/LogBox/__tests__/LogBox-integration-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,56 +11,44 @@
import {
DoesNotUseKey,
FragmentWithProp,
ManualConsoleError,
ManualConsoleErrorWithStack,
} from './__fixtures__/ReactWarningFixtures';
import * as React from 'react';

const LogBoxData = require('../Data/LogBoxData');
const TestRenderer = require('react-test-renderer');

const installLogBox = () => {
const LogBox = require('../LogBox');
const ExceptionsManager = require('../../Core/ExceptionsManager.js');

const installLogBox = () => {
const LogBox = require('../LogBox').default;
LogBox.install();
};

const uninstallLogBox = () => {
const LogBox = require('../LogBox');
const LogBox = require('../LogBox').default;
LogBox.uninstall();
};

const BEFORE_SLASH_RE = /(?:\/[a-zA-Z]+\/)(.+?)(?:\/.+)\//;

const cleanPath = message => {
return message.replace(BEFORE_SLASH_RE, '/path/to/');
};

const cleanLog = logs => {
return logs.map(log => {
return {
...log,
componentStack: log.componentStack.map(stack => ({
...stack,
fileName: cleanPath(stack.fileName),
})),
};
});
};

// TODO(T71117418): Re-enable skipped LogBox integration tests once React component
// stack frames are the same internally and in open source.
// eslint-disable-next-line jest/no-disabled-tests
describe.skip('LogBox', () => {
// TODO: we can remove all the symetric matchers once OSS lands component stack frames.
// For now, the component stack parsing differs in ways we can't easily detect in this test.
describe('LogBox', () => {
const {error, warn} = console;
const mockError = jest.fn();
const mockWarn = jest.fn();

beforeEach(() => {
jest.resetModules();
jest.restoreAllMocks();
jest.spyOn(console, 'error').mockImplementation(() => {});

mockError.mockClear();
mockWarn.mockClear();

// Reset ExceptionManager patching.
if (console._errorOriginal) {
console._errorOriginal = null;
}
(console: any).error = mockError;
(console: any).warn = mockWarn;
});
Expand All @@ -79,7 +67,10 @@ describe.skip('LogBox', () => {
// so we can assert on what React logs.
jest.spyOn(console, 'error');

const output = TestRenderer.create(<DoesNotUseKey />);
let output;
TestRenderer.act(() => {
output = TestRenderer.create(<DoesNotUseKey />);
});

// The key error should always be the highest severity.
// In LogBox, we expect these errors to:
Expand All @@ -88,16 +79,37 @@ describe.skip('LogBox', () => {
// - Pass to console.error, with a "Warning" prefix so it does not pop a RedBox.
expect(output).toBeDefined();
expect(mockWarn).not.toBeCalled();
expect(console.error.mock.calls[0].map(cleanPath)).toMatchSnapshot(
'Log sent from React',
);
expect(cleanLog(spy.mock.calls[0])).toMatchSnapshot('Log added to LogBox');
expect(mockError.mock.calls[0].map(cleanPath)).toMatchSnapshot(
'Log passed to console error',
);
expect(console.error).toBeCalledTimes(1);
expect(console.error.mock.calls[0]).toEqual([
'Each child in a list should have a unique "key" prop.%s%s See https://react.dev/link/warning-keys for more information.%s',
'\n\nCheck the render method of `DoesNotUseKey`.',
'',
expect.stringMatching('at DoesNotUseKey'),
]);
expect(spy).toHaveBeenCalledWith({
level: 'warn',
category: expect.stringContaining(
'Warning: Each child in a list should have a unique',
),
componentStack: expect.anything(),
componentStackType: 'stack',
message: {
content:
'Warning: Each child in a list should have a unique "key" prop.\n\nCheck the render method of `DoesNotUseKey`. See https://react.dev/link/warning-keys for more information.',
substitutions: [
{length: 45, offset: 62},
{length: 0, offset: 107},
],
},
});

// The Warning: prefix is added due to a hack in LogBox to prevent double logging.
expect(mockError.mock.calls[0][0].startsWith('Warning: ')).toBe(true);
// We also interpolate the string before passing to the underlying console method.
expect(mockError.mock.calls[0]).toEqual([
expect.stringMatching(
'Warning: Each child in a list should have a unique "key" prop.\n\nCheck the render method of `DoesNotUseKey`. See https://react.dev/link/warning-keys for more information.\n at ',
),
]);
});

it('integrates with React and handles a fragment warning in LogBox', () => {
Expand All @@ -108,7 +120,10 @@ describe.skip('LogBox', () => {
// so we can assert on what React logs.
jest.spyOn(console, 'error');

const output = TestRenderer.create(<FragmentWithProp />);
let output;
TestRenderer.act(() => {
output = TestRenderer.create(<FragmentWithProp />);
});

// The fragment warning is not as severe. For this warning we don't want to
// pop open a dialog, so we show a collapsed error UI.
Expand All @@ -118,15 +133,125 @@ describe.skip('LogBox', () => {
// - Pass to console.error, with a "Warning" prefix so it does not pop a RedBox.
expect(output).toBeDefined();
expect(mockWarn).not.toBeCalled();
expect(console.error.mock.calls[0].map(cleanPath)).toMatchSnapshot(
'Log sent from React',
);
expect(cleanLog(spy.mock.calls[0])).toMatchSnapshot('Log added to LogBox');
expect(mockError.mock.calls[0].map(cleanPath)).toMatchSnapshot(
'Log passed to console error',
);
expect(console.error).toBeCalledTimes(1);
expect(console.error.mock.calls[0]).toEqual([
'Invalid prop `%s` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.%s',
'invalid',
expect.stringMatching('at FragmentWithProp'),
]);
expect(spy).toHaveBeenCalledWith({
level: 'warn',
category: expect.stringContaining('Warning: Invalid prop'),
componentStack: expect.anything(),
componentStackType: expect.stringMatching(/(stack|legacy)/),
message: {
content:
'Warning: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.',
substitutions: [{length: 7, offset: 23}],
},
});

// The Warning: prefix is added due to a hack in LogBox to prevent double logging.
// We also interpolate the string before passing to the underlying console method.
expect(mockError.mock.calls[0]).toEqual([
expect.stringMatching(
'Warning: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.\n at FragmentWithProp',
),
]);
});

it('handles a manual console.error without a component stack in LogBox', () => {
const LogBox = require('../LogBox').default;
const spy = jest.spyOn(LogBox, 'addException');
installLogBox();

// console.error handling depends on installing the ExceptionsManager error reporter.
ExceptionsManager.installConsoleErrorReporter();

// Spy console.error after LogBox is installed
// so we can assert on what React logs.
jest.spyOn(console, 'error');

let output;
TestRenderer.act(() => {
output = TestRenderer.create(<ManualConsoleError />);
});

// Manual console errors should show a collapsed error dialog.
// When there is no component stack, we expect these errors to:
// - Go to the LogBox patch and fall through to console.error.
// - Get picked up by the ExceptionsManager console.error override.
// - Get passed back to LogBox via addException (non-fatal).
expect(output).toBeDefined();
expect(mockWarn).not.toBeCalled();
expect(spy).toBeCalledTimes(1);
expect(console.error).toBeCalledTimes(1);
expect(console.error.mock.calls[0]).toEqual(['Manual console error']);
expect(spy).toHaveBeenCalledWith({
id: 1,
isComponentError: false,
isFatal: false,
name: 'console.error',
originalMessage: 'Manual console error',
message: 'console.error: Manual console error',
extraData: expect.anything(),
componentStack: null,
stack: expect.anything(),
});

// No Warning: prefix is added due since this is falling through.
expect(mockError.mock.calls[0]).toEqual(['Manual console error']);
});

it('handles a manual console.error with a component stack in LogBox', () => {
const spy = jest.spyOn(LogBoxData, 'addLog');
installLogBox();

// console.error handling depends on installing the ExceptionsManager error reporter.
ExceptionsManager.installConsoleErrorReporter();

// Spy console.error after LogBox is installed
// so we can assert on what React logs.
jest.spyOn(console, 'error');

let output;
TestRenderer.act(() => {
output = TestRenderer.create(<ManualConsoleErrorWithStack />);
});

// Manual console errors should show a collapsed error dialog.
// When there is a component stack, we expect these errors to:
// - Go to the LogBox patch and be detected as a React error.
// - Check the warning filter to see if there is a fiter setting.
// - Call console.error with the parsed error.
// - Get picked up by ExceptionsManager console.error override.
// - Log to console.error.
expect(output).toBeDefined();
expect(mockWarn).not.toBeCalled();
expect(console.error).toBeCalledTimes(1);
expect(spy).toBeCalledTimes(1);
expect(console.error.mock.calls[0]).toEqual([
expect.stringContaining(
'Manual console error\n at ManualConsoleErrorWithStack',
),
]);
expect(spy).toHaveBeenCalledWith({
level: 'warn',
category: expect.stringContaining('Warning: Manual console error'),
componentStack: expect.anything(),
componentStackType: 'stack',
message: {
content: 'Warning: Manual console error',
substitutions: [],
},
});

// The Warning: prefix is added due to a hack in LogBox to prevent double logging.
expect(mockError.mock.calls[0][0].startsWith('Warning: ')).toBe(true);
// We also interpolate the string before passing to the underlying console method.
expect(mockError.mock.calls[0]).toEqual([
expect.stringMatching(
'Warning: Manual console error\n at ManualConsoleErrorWithStack',
),
]);
});
});
Loading
Loading