[expect]: add empty string check to toThrow matcher

[mattphillips]

Summary

Add a truthy check of the given value when performing toThrowMatchingStringOrRegexp in the toThrow matcher.

@SimenB @thymikee I'm not sure if you want to solve this another way but this seemed this simplest, what do you guys think?

Fixes #6805

Test plan

Added unit tests for empty string.

[thymikee]

LGTM

[SimenB]

What about fixing the pattern instead? This will give weird results for falsey values outside of empty strings (such as 0), I think

[mattphillips]

@SimenB I've just updated it to default to ^$ for empty strings

[mattphillips]

Not sure why the tests are failing now! I'd say something maybe depends on providing empty strings to the escapeStrForRegex??

[SimenB]

@mattphillips it doesn't fail locally?

[mattphillips]

@SimenB seems like the failing tests are:

    testPathsMatching
      ✕ finds tests matching a pattern via testRegex (89ms)
      ✕ finds tests matching a pattern via testMatch (30ms)

What is weird is that they are failing for me locally on master too (I did a clean clone/install/test)

yarn jest packages/jest-cli/src/__tests__/SearchSource.test.js

[SimenB]

@mattphillips that fails for me locally as well, not sure why.. However, the only test failing on CI is e2e/__tests__/find_related_files.test.js, mind fixing that? If CI is happy, I'm happy 🙂

[mattphillips]

@SimenB I've just spent a chunk of time investigating why this test is failing and couldn't figure it out, so many moving parts 🤣

I've instead moved the check to the matcher itself as we know for sure this is the behaviour we want - let me know what you think?

[SimenB]

shouldn't this text include 'apple'?

[mattphillips]

The test file defines a custom error with a fixed message for the snapshot - I’ve just reused that which is why it doesn’t say apple

[SimenB]

seems silly, doesn't it? Is the error inconsistent if we drop the fixed message?

[mattphillips]

Yeah it does a bit! I’ll give it a go and see what happens, maybe it was giving inconsistent line numbers 🤷

[mattphillips]

Ah I just noticed this comment above the custom error/stack:

Custom Error class because node versions have different stack trace strings.

[SimenB]

is it still valid?

[SimenB]

Also, toThrowErrorMatchingSnapshot only snapshots the message, not stack

[SimenB]

@mattphillips what's the status here? To fix your failing test locally, run watchman watch-del-all

[mattphillips]

Not needed see #6805 (comment)

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.