Skip to content

Commit

Permalink
Improved toWarnDev matcher to avoid swallowing errors
Browse files Browse the repository at this point in the history
While writing tests for unsafe async warnings, I noticed that in certain cases, errors were swallowed by the toWarnDev matcher and resulted in confusing test failures. For example, if an error prevented the code being tested from logging an expected warning- the test would fail saying that the warning hadn't been logged rather than reporting the unexpected error. I think a better approach for this is to always treat caught errors as the highest-priority reason for failing a test.

I reran all of the test cases for this matcher that I originally ran with PR facebook#11786 and ensured they all still pass.
  • Loading branch information
bvaughn committed Jan 23, 2018
1 parent 4d65408 commit 29b31bc
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions scripts/jest/matchers/toWarnDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ const createMatcherFor = consoleMethod =>
// Restore the unspied method so that unexpected errors fail tests.
console[consoleMethod] = originalMethod;

// Any unexpected Errors thrown by the callback should fail the test.
// This should take precedence since unexpected errors could block warnings.
if (caughtError) {
throw caughtError;
}

// Any unexpected warnings should be treated as a failure.
if (unexpectedWarnings.length > 0) {
return {
Expand All @@ -89,11 +95,6 @@ const createMatcherFor = consoleMethod =>
};
}

// Any unexpected Errors thrown by the callback should fail the test.
if (caughtError) {
throw caughtError;
}

return {pass: true};
}
} else {
Expand Down

0 comments on commit 29b31bc

Please sign in to comment.