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

Should we handle resolves and rejects for matchers? #29

Open
jo-sm opened this issue May 24, 2023 · 0 comments
Open

Should we handle resolves and rejects for matchers? #29

jo-sm opened this issue May 24, 2023 · 0 comments
Labels
question Further information is requested

Comments

@jo-sm
Copy link
Contributor

jo-sm commented May 24, 2023

When dealing with promise assertions in Jest you can write code that simplifies the need to first create a variable and then assert against it, by using expect(...).resolves and .rejects:

//  with .resolves
expect(Promise.resolve('something-cool')).resolves.toBe('something-cool');

// without (assuming we're in an async fn
const result = await Promise.resolve('something-cool');
expect(result).toBe('something-cool');

In #27 we'll already fix negation for matchers (expect(...).not), and Jest seems to consider resolves and rejects as the same as .not, as modifiers, so we could in theory extend the functionality to support more than just not.

Do we want to support it?

On the one hand, it would be nice to support as much as possible in Jest, and since not is considered a modifier, it would be nice to allow other modifiers for is bodies.

On the other hand, it could make things more complicated as a cljest user. For example, with the current approach of generating code for non-matcher is bodies, we wouldn't be able to support generically resolving or rejecting the promise, because we take whatever form is passed into is and wrap it in a do block and assert that the result is truthy, and so if we didn't change that the caller would need to keep in mind that their assertion is a Jest matcher to be able to use resolves or rejects, which is a detail that is ideally avoided.

One possible way to make it work for most cases would be to modify how is works now, and instead of wrapping the code in do we look at the resolved symbol and pass that as an argument to the underlying expect call, along with the argument passed to the resolved symbol, kind of like this:

(is (resolves (= (js/Promise.resolve "something-cool") "something-cool")))

;; above would basically become
expect(Promise.resolve("something-cool")).resolves.cljest__is(cljs.core.__EQ__, "something-cool", formatterFn)

And we could even expand it to chain the modifiers like what's allowed in vanilla:

(is (not (resolves (= promise "something-cool"))))

Based on my initial thoughts, this would work in a lot of cases, but it wouldn't work for macros. Is it a worthwhile tradeoff? Would people even use it?

@jo-sm jo-sm added the question Further information is requested label May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

No branches or pull requests

1 participant