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

[tests] Disallow unasserted console.log #28708

Merged
merged 3 commits into from
Apr 1, 2024
Merged

[tests] Disallow unasserted console.log #28708

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9063df3
[tests] Disallow unasserted console.log
rickhanlonii Apr 1, 2024
1e982d2
remove test log
rickhanlonii Apr 1, 2024
b98343b
Fix withoutStack, add tests
rickhanlonii Apr 1, 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
3 changes: 3 additions & 0 deletions packages/react/src/__tests__/ReactStrictMode-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ describe('ReactStrictMode', () => {
});

it('should appear in the client component stack', async () => {
// TODO REMOVE AFTER TESTING THIS FAILS ON PR
console.log('hit');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm expecting the test to fail just to confirm the CI envar is set properly, then I'll remove this.


function Foo() {
return <div ariaTypo="" />;
}
Expand Down
95 changes: 50 additions & 45 deletions scripts/jest/matchers/toWarnDev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,52 +178,56 @@ const createMatcherFor = (consoleMethod, matcherName) =>
};
}

if (typeof withoutStack === 'number') {
// We're expecting a particular number of warnings without stacks.
if (withoutStack !== warningsWithoutComponentStack.length) {
return {
message: () =>
`Expected ${withoutStack} warnings without a component stack but received ${warningsWithoutComponentStack.length}:\n` +
warningsWithoutComponentStack.map(warning =>
this.utils.printReceived(warning)
),
pass: false,
};
}
} else if (withoutStack === true) {
// We're expecting that all warnings won't have the stack.
// If some warnings have it, it's an error.
if (warningsWithComponentStack.length > 0) {
return {
message: () =>
`Received warning unexpectedly includes a component stack:\n ${this.utils.printReceived(
warningsWithComponentStack[0]
)}\nIf this warning intentionally includes the component stack, remove ` +
`{withoutStack: true} from the ${matcherName}() call. If you have a mix of ` +
`warnings with and without stack in one ${matcherName}() call, pass ` +
`{withoutStack: N} where N is the number of warnings without stacks.`,
pass: false,
};
}
} else if (withoutStack === false || withoutStack === undefined) {
// We're expecting that all warnings *do* have the stack (default).
// If some warnings don't have it, it's an error.
if (warningsWithoutComponentStack.length > 0) {
return {
message: () =>
`Received warning unexpectedly does not include a component stack:\n ${this.utils.printReceived(
warningsWithoutComponentStack[0]
)}\nIf this warning intentionally omits the component stack, add ` +
`{withoutStack: true} to the ${matcherName} call.`,
pass: false,
};
if (consoleMethod === 'log') {
// We don't expect any console.log calls to have a stack.
} else if (typeof withoutStack === 'number') {
if (typeof withoutStack === 'number') {
// We're expecting a particular number of warnings without stacks.
if (withoutStack !== warningsWithoutComponentStack.length) {
return {
message: () =>
`Expected ${withoutStack} warnings without a component stack but received ${warningsWithoutComponentStack.length}:\n` +
warningsWithoutComponentStack.map(warning =>
this.utils.printReceived(warning)
),
pass: false,
};
}
} else if (withoutStack === true) {
// We're expecting that all warnings won't have the stack.
// If some warnings have it, it's an error.
if (warningsWithComponentStack.length > 0) {
return {
message: () =>
`Received warning unexpectedly includes a component stack:\n ${this.utils.printReceived(
warningsWithComponentStack[0]
)}\nIf this warning intentionally includes the component stack, remove ` +
`{withoutStack: true} from the ${matcherName}() call. If you have a mix of ` +
`warnings with and without stack in one ${matcherName}() call, pass ` +
`{withoutStack: N} where N is the number of warnings without stacks.`,
pass: false,
};
}
} else if (withoutStack === false || withoutStack === undefined) {
// We're expecting that all warnings *do* have the stack (default).
// If some warnings don't have it, it's an error.
if (warningsWithoutComponentStack.length > 0) {
return {
message: () =>
`Received warning unexpectedly does not include a component stack:\n ${this.utils.printReceived(
warningsWithoutComponentStack[0]
)}\nIf this warning intentionally omits the component stack, add ` +
`{withoutStack: true} to the ${matcherName} call.`,
pass: false,
};
}
} else {
throw Error(
`The second argument for ${matcherName}(), when specified, must be an object. It may have a ` +
`property called "withoutStack" whose value may be undefined, boolean, or a number. ` +
`Instead received ${typeof withoutStack}.`
);
}
} else {
throw Error(
`The second argument for ${matcherName}(), when specified, must be an object. It may have a ` +
`property called "withoutStack" whose value may be undefined, boolean, or a number. ` +
`Instead received ${typeof withoutStack}.`
);
}

if (lastWarningWithMismatchingFormat !== null) {
Expand Down Expand Up @@ -309,4 +313,5 @@ const createMatcherFor = (consoleMethod, matcherName) =>
module.exports = {
toWarnDev: createMatcherFor('warn', 'toWarnDev'),
toErrorDev: createMatcherFor('error', 'toErrorDev'),
toLogDev: createMatcherFor('log', 'toLogDev'),
};
25 changes: 23 additions & 2 deletions scripts/jest/setupTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,28 +116,37 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
.join('\n')}`
);

const type = methodName === 'log' ? 'log' : 'warning';
const message =
`Expected test not to call ${chalk.bold(
`console.${methodName}()`
)}.\n\n` +
'If the warning is expected, test for it explicitly by:\n' +
`If the ${type} is expected, test for it explicitly by:\n` +
`1. Using the ${chalk.bold('.' + expectedMatcher + '()')} ` +
`matcher, or...\n` +
`2. Mock it out using ${chalk.bold(
'spyOnDev'
)}(console, '${methodName}') or ${chalk.bold(
'spyOnProd'
)}(console, '${methodName}'), and test that the warning occurs.`;
)}(console, '${methodName}'), and test that the ${type} occurs.`;

throw new Error(`${message}\n\n${messages.join('\n\n')}`);
}
};

const unexpectedErrorCallStacks = [];
const unexpectedWarnCallStacks = [];
const unexpectedLogCallStacks = [];

const errorMethod = patchConsoleMethod('error', unexpectedErrorCallStacks);
const warnMethod = patchConsoleMethod('warn', unexpectedWarnCallStacks);
let logMethod;

// Only assert console.log isn't called in CI so you can debug tests in DEV.
// The matchers will still work in DEV, so you can assert locally.
if (process.env.CI) {
logMethod = patchConsoleMethod('log', unexpectedLogCallStacks);
}

const flushAllUnexpectedConsoleCalls = () => {
flushUnexpectedConsoleCalls(
Expand All @@ -152,13 +161,25 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
'toWarnDev',
unexpectedWarnCallStacks
);
if (logMethod) {
flushUnexpectedConsoleCalls(
logMethod,
'log',
'toLogDev',
unexpectedLogCallStacks
);
unexpectedLogCallStacks.length = 0;
}
unexpectedErrorCallStacks.length = 0;
unexpectedWarnCallStacks.length = 0;
};

const resetAllUnexpectedConsoleCalls = () => {
unexpectedErrorCallStacks.length = 0;
unexpectedWarnCallStacks.length = 0;
if (logMethod) {
unexpectedLogCallStacks.length = 0;
}
};

beforeEach(resetAllUnexpectedConsoleCalls);
Expand Down