Jest: 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.

[vitaly-t]

@mattphillips Contained???!!! Holly!!! That's kind of against how toThrow works in all other test frameworks, I was totally not expecting this.... toThrow everywhere else is the exact match, not just a contained error text.

So, if I want to test the exact error/message, how should I do it then?

[mattphillips]

Aye I agree it's not very intuitive! Although I can see the usefulness of being able to assert on a substring as you might not own the whole error message and it makes sense to just assert on the bit you care about.

Perhaps this behaviour should be its own matcher @SimenB @thymikee ?

[thymikee]

Nah, I like the current behavior and wouldn't change it. It's been like this forever and users didn't mind.

[webstacker]

@vitaly-t @mattphillips I've just been stung by this too, I expected that if I supply a string it matches it exactly. For simple strings it's not too difficult to pass a regex, but anything that requires escape characters it gets tedious e.g.

it.only('should match exact string', () => {
    // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
    function escapeRegExp(string) {
        return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
    }

    function getFile(path) {
        // some call to get the file causes an error...
        throw new Error(`File not found ("${path}")`);
    }

    const filePath = path.join(__dirname, 'somefile.txt'); // C:\folder\subfolder\somefile.txt
    const exactError = `File not found ("${filePath}")`;

    expect(() => {
        getFile(filePath);
    }).toThrowError(`File not found`); // expect to fail but passes

    expect(() => {
        getFile(filePath);
    }).toThrowError(new RegExp(`^${escapeRegExp(exactError)}$`)); // passes as expected
});

I'll write a custom matcher "toThrowExactError" to avoid having to escape things all over the place, but it would be nice to be able to pass a string and specify it should be an exact match. I'd argue this should be the default behaviour.

I didn't help myself by somehow reading these docs with don't mention "contained":

If you want to test that a specific error gets thrown, you can provide an argument to toThrow. The argument can be a string for the error message

[SimenB]

Wanna send a PR making the docs highlight this?

[mattphillips]

Closing as the docs have now been updated in: #7621 and reflect that toThrow(string) uses substring matching so expect(() => throw 'something').toThrow('') should pass as all strings include an empty string xD