Fix SSR integration test swallowing lack of expected error #10331
Merged
gaearon merged 2 commits intofacebook:masterfrom Jul 31, 2017
Merged
Fix SSR integration test swallowing lack of expected error #10331gaearon merged 2 commits intofacebook:masterfrom
gaearon merged 2 commits intofacebook:masterfrom
Conversation
sebmarkbage
approved these changes
Jul 31, 2017
Contributor
sebmarkbage
left a comment
sebmarkbage
left a comment
There was a problem hiding this comment.
Another instance of jestjs/jest#3917 gone wrong.
Previously, even if an error was not thrown, the assertion about this was ignored. This fix exposes two failing cases in new SSR that weren't failing in Stack.
This fixes the error cases that were silently failing before but are now exposed.
gaearon
force-pushed
the
fix-ssr-error-assertions
branch
from
8c42e82 to
b7d6e7f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We have some tests that check that expected errors are thrown.
These tests use a helper called
itThrowsWhenRendering.Currently, there is a bug in this helper. When there is no error (but an error is expected), it tries to fail the test with
expect(false).toBe('The promise resolved and should not have.'). However thecatch()block catches and swallows this exception.The fix is to attach the error handler to the original Promise rather than to the one after our handler.
This fix exposes two missing expected errors in new SSR related to context. The second commit fixes them by adding the respective invariants.
To future proof this, I want to assert on specific error messages instead. I will do so in a follow up PR.