From e9ae2c8f352289a8c942b4271b9afe7af68dd5c0 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 1 Apr 2024 10:50:48 -0400 Subject: [PATCH] Clean up console.log tests (#28693) Followups to https://github.com/facebook/react/pull/28680 One of these test don't need to use `console.log`. The others are specifically testing `console.log` behavior, so I added a comment. --- .../src/__tests__/ReactFlight-test.js | 4 +++ .../src/__tests__/ReactDOMComponent-test.js | 19 ++++++------ .../src/__tests__/ReactStrictMode-test.js | 31 +++++-------------- 3 files changed, 20 insertions(+), 34 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 97e29ced26679..55956ddc93ecd 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2108,6 +2108,7 @@ describe('ReactFlight', () => { throw new Error('err'); } + // These tests are specifically testing console.log. // Assign to `mockConsoleLog` so we can still inspect it when `console.log` // is overridden by the test modules. The original function will be restored // after this test finishes by `jest.restoreAllMocks()`. @@ -2127,6 +2128,9 @@ describe('ReactFlight', () => { transport = ReactNoopFlightServer.render({root: }); }).toErrorDev('err'); + expect(mockConsoleLog).toHaveBeenCalledTimes(1); + expect(mockConsoleLog.mock.calls[0][0]).toBe('hi'); + expect(mockConsoleLog.mock.calls[0][1].prop).toBe(123); mockConsoleLog.mockClear(); // The error should not actually get logged because we're not awaiting the root diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 4f31ee906d7af..1d0c3581387da 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -17,6 +17,8 @@ describe('ReactDOMComponent', () => { const ReactFeatureFlags = require('shared/ReactFeatureFlags'); let act; + let assertLog; + let Scheduler; beforeEach(() => { jest.resetModules(); @@ -24,7 +26,9 @@ describe('ReactDOMComponent', () => { ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); + Scheduler = require('scheduler'); act = require('internal-test-utils').act; + assertLog = require('internal-test-utils').assertLog; }); afterEach(() => { @@ -1611,7 +1615,6 @@ describe('ReactDOMComponent', () => { }); it('should work error event on element', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); await act(() => { @@ -1620,7 +1623,7 @@ describe('ReactDOMComponent', () => { console.log('onError called')} + onError={e => Scheduler.log('onError called')} /> , ); @@ -1631,8 +1634,7 @@ describe('ReactDOMComponent', () => { container.getElementsByTagName('source')[0].dispatchEvent(errorEvent); if (__DEV__) { - expect(console.log).toHaveBeenCalledTimes(1); - expect(console.log.mock.calls[0][0]).toContain('onError called'); + assertLog(['onError called']); } }); @@ -1921,7 +1923,6 @@ describe('ReactDOMComponent', () => { }); it('should work load and error events on element in SVG', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); await act(() => { @@ -1929,8 +1930,8 @@ describe('ReactDOMComponent', () => { console.log('onError called')} - onLoad={e => console.log('onLoad called')} + onError={e => Scheduler.log('onError called')} + onLoad={e => Scheduler.log('onLoad called')} /> , ); @@ -1946,9 +1947,7 @@ describe('ReactDOMComponent', () => { container.getElementsByTagName('image')[0].dispatchEvent(loadEvent); if (__DEV__) { - expect(console.log).toHaveBeenCalledTimes(2); - expect(console.log.mock.calls[0][0]).toContain('onError called'); - expect(console.log.mock.calls[1][0]).toContain('onLoad called'); + assertLog(['onError called', 'onLoad called']); } }); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index cce952e39e222..588b65f139ebe 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -1146,12 +1146,17 @@ describe('context legacy', () => { React = require('react'); ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; + + // These tests are specifically testing console.log. + spyOnDevAndProd(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + console.log.mockRestore(); }); if (ReactFeatureFlags.consoleManagedByDevToolsDuringStrictMode) { it('does not disable logs for class double render', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { render() { @@ -1179,8 +1184,6 @@ describe('context legacy', () => { }); it('does not disable logs for class double ctor', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { constructor(props) { @@ -1211,8 +1214,6 @@ describe('context legacy', () => { }); it('does not disable logs for class double getDerivedStateFromProps', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { state = {}; @@ -1244,8 +1245,6 @@ describe('context legacy', () => { }); it('does not disable logs for class double shouldComponentUpdate', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { state = {}; @@ -1285,8 +1284,6 @@ describe('context legacy', () => { }); it('does not disable logs for class state updaters', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let inst; let count = 0; class Foo extends React.Component { @@ -1323,8 +1320,6 @@ describe('context legacy', () => { }); it('does not disable logs for function double render', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; function Foo() { count++; @@ -1350,8 +1345,6 @@ describe('context legacy', () => { }); } else { it('disable logs for class double render', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { render() { @@ -1379,8 +1372,6 @@ describe('context legacy', () => { }); it('disables logs for class double ctor', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { constructor(props) { @@ -1411,8 +1402,6 @@ describe('context legacy', () => { }); it('disable logs for class double getDerivedStateFromProps', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { state = {}; @@ -1444,8 +1433,6 @@ describe('context legacy', () => { }); it('disable logs for class double shouldComponentUpdate', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; class Foo extends React.Component { state = {}; @@ -1484,8 +1471,6 @@ describe('context legacy', () => { }); it('disable logs for class state updaters', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let inst; let count = 0; class Foo extends React.Component { @@ -1522,8 +1507,6 @@ describe('context legacy', () => { }); it('disable logs for function double render', async () => { - spyOnDevAndProd(console, 'log').mockImplementation(() => {}); - let count = 0; function Foo() { count++;