Invalid toThrow verification

[vitaly-t]

🐛 Bug Report

.toThrow() fails to check correctly against an empty string or a string with one symbol.

To Reproduce

it('must fail when compared against an empty string', () => {
    expect(() => {
        throw 'something';
     }).toThrow(''); // PASS
});

it('must fail against a string with one letter', () => {
    expect(() => {
        throw 'something';
     }).toThrow('a'); // PASS
});

Expected behavior

The two cases above are supposed to fail. But instead, they both succeed.

Curiously, if in the second case we use more than one letter, only then the test fails.

[mattphillips]

Hey @vitaly-t are you able to supply the versions of node/jest you are using? You can find them by running: npx envinfo --preset jest

I've just ran the above tests on the latest version of Jest (23.4.2) and am seeing that the first tests passes and the second (correctly) fails.

Looks like this is a bug!

[SimenB]

The error is pretty ugly as well, OTTOMH not sure how we can avoid the double frame

[SimenB]

The first one passes since we convert it to a regex (empty regexp is the same as /(?:)/, so it matches anything)

https://github.com/facebook/jest/blob/c9893d2f1cdbc98655ca446d2a49f8c77a42b4be/packages/expect/src/to_throw_matchers.js#L53-L55

'something'.match(new RegExp('')) // returns [''], which is considered a match

@thymikee @rickhanlonii should we drop the regexp conversion for empty strings? My guess the current behaviour exists to allow substring matches, but using .includes won't help for empty strings.

[thymikee]

I think it would be sane to drop it

[SimenB]

Is it possible to make a regex be a literal empty string instead of (?:)?

EDIT: Could do /^$/ but that seems a bit hacky. Maybe OK? Or throw and ask consumers to either drop the argument or pass /^$/ to the matcher themselves for empty strings?

[thymikee]

Not sure what you mean? Can't we just add a case for empty string for better error message?

[vitaly-t]

@thymikee Beside the empty string, as per the initial report, I am getting consistent failure with this - .toThrow('a'); // PASS, which supposed to fail, but it passes. And when I make it longer than 1 symbol, only then it fails as it should.

[mattphillips]

@vitaly-t without knowing your setup or a reproduction we aren't able to look into why the second test is passing for you. Are you able to create a simple repro? Here is a link to repl with the test successfully failing: https://repl.it/repls/LastSimpleMarkuplanguage

[SimenB]

I also cannot reproduce the second one (beyond being unhappy with the double code frame, but it works)

[vitaly-t]

Here's that evil case reproduced:

it('must fail against a string with one letter', () => {
    expect(() => {
        throw 'bla';
     }).toThrow('a'); // PASS
});

What's the difference? - you guys tell me, I don't understand it myself.

But you can copy and paste exactly into that other test, and you will see ;)

@mattphillips

[mattphillips]

@vitaly-t that test should pass as toThrow takes a regex or a string to check if it is included in the error and a is in bla

See the docs:

If you want to test that a specific error gets thrown, you can provide an argument to toThrow. The argument can be a string that should be contained in the error message, a class for the error, or a regex that should match the error message. For example, let's say that drinkFlavor is coded like this:

https://jestjs.io/docs/en/expect#tothrowerror

[vitaly-t]

Here, I have updated it - https://repl.it/@vitaly_t1/LastSimpleMarkuplanguage so you can see the failure.