Skip to content

Commit

Permalink
Re-enable integration tests (#46639)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #46639

These tests were skipped when we were switching to component stacks, which also hid a bug later in the stack. Re-enable them.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D63349616

fbshipit-source-id: ccde7d5bb3fcd9a27adf4af2068a160f02f7432a
  • Loading branch information
rickhanlonii authored and blakef committed Oct 7, 2024
1 parent 9ae812c commit bf40710
Showing 1 changed file with 66 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ const LogBoxData = require('../Data/LogBoxData');
const TestRenderer = require('react-test-renderer');

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

LogBox.install();
};

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

Expand All @@ -46,21 +46,20 @@ const cleanLog = logs => {
});
};

// 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();

(console: any).error = mockError;
(console: any).warn = mockWarn;
});
Expand All @@ -79,7 +78,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 +90,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].map(cleanPath)).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 +131,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 +144,30 @@ 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].map(cleanPath)).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.
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: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.\n at FragmentWithProp',
),
]);
});
});

0 comments on commit bf40710

Please sign in to comment.