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.

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

Add some sugar to simplify test assertions #11626

Closed
gaearon opened this issue Nov 22, 2017 · 3 comments
Closed

Add some sugar to simplify test assertions #11626

gaearon opened this issue Nov 22, 2017 · 3 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Nov 22, 2017

I made tests a bit too verbose in #11616.
Might be a good time to revisit how we write them now.

There's a few separate issues:

  • Asserting on warning messages is too verbose. Need a DEV block, call spyOnDev, normalize stack, etc. Ideally I wish there was a higher level helper that lets us "push" expected warning assertions and validate them as they happen. It should handle component stacks automatically without having to copy-paste the normalization helper everywhere.

  • In some cases I added actual duplication in Run Jest in production mode #11616, in particular for cases that throw in development but don't throw in production. Maybe need a toThrowInDev()? Although on the other hand this pattern needs to be highly visible so maybe extracting common code in a closure is good enough.

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 22, 2017

Here's one proposal:

expect(
  fn: () => void,
).toWarnInDev(
  pattern: string | Array<string>,
);

for the most common console.error case. If pattern is a string, it expects a single warning. If it is an array, it expects multiple warnings. All patterns are normalized so it's easy to match the component stack traces.

The nice thing about it is it clearly scopes which code is supposed to warn. The downside of this is that it's harder to assert on things that both throw and warn because you'd have to wrap the code twice (or else add a new helper).

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 22, 2017

Another thing to consider. We currently globally mock ReactFiberErrorLogger because it is too noisy. We could remove that global mock but then every test that throws inside React call stack would also log a warning (which it does for consumers in open source). Maybe we could solve this with an extra matcher. Or make it configurable somehow.

I fixed this.

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 6, 2018

Fixed in #11786.

@gaearon gaearon closed this as completed Jan 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants