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

incrementAssertionCount throws exception "expect is not defined" #422

Merged
merged 3 commits into from Nov 27, 2017

Conversation

OrangeBean
Copy link
Contributor

This is the fix for issue #421

I'm using Frisby to test REST API. I'm replacing Jest with Gauge. It turns out Frisby throws exception if there is no Jasmine expect defined.

Kirill Zaskalkin added 2 commits November 27, 2017 12:10
- If Frisby executed without Jasmine/Jest (e.g. using Gauge)
incrementAssertionCount throws exception "expect is not defined".
@@ -11,7 +11,7 @@ const utils = require('./utils');
* Jasmine and others
*/
function incrementAssertionCount() {
if (_.isFunction(expect)) {
if (typeof expect != 'undefined' && _.isFunction(expect)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OrangeBean

If expect is undefined then _.isFunction(expect) returns false.
https://lodash.com/docs/#isFunction
So, I think there is no effect.

In this issue, what is set for expect ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect is not defined anywhere as there is no Jasmine. The issue is that it is not possible to call function with undefined variable as a parameter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OrangeBean
Thanks. I understand.

use !== is more better.

@vlucas vlucas merged commit ef9d413 into vlucas:master Nov 27, 2017
@vlucas
Copy link
Owner

vlucas commented Nov 27, 2017

Frisby is only officially supported with Jest, but the intent of the code here remains the same, so I just merged it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants