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

Options to not close browser on finish and/or on error. #5

Merged
merged 3 commits into from
Mar 24, 2015

Conversation

dmwyatt
Copy link
Contributor

@dmwyatt dmwyatt commented Mar 23, 2015

I know in mantoni/mochify.js#77 you metioned closeOnFailure, but but closeOnFinish was easy to implement and I can foresee use cases for exploring problems.

As you mentioned on the other issue, this defaults to closing the browser when done, but if you set closeOnFinish/closeOnError option to false, it will leave browser open.

@mantoni
Copy link
Owner

mantoni commented Mar 23, 2015

Very nice. I like the option names and the separation into two options makes sense as well.

Regarding closeOnFinish: This sounds a bit like it's never going to close regardless of whether an error occurred or not. It could be either renamed to closeOnSuccess or it could implicitly set closeOnError to false as well.

From looking at the diff, I think there should be an else case for all the new ifs that invoke the callback function immediately.

What do you think?

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Mar 24, 2015

Something like this?

    if (err && context.closeOnError) {
      close(context, function () {
        callback(err);
      });
      return;
    } else if(err) {
      callback(err);
    }

@mantoni
Copy link
Owner

mantoni commented Mar 24, 2015

Yes. Exactly.

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Mar 24, 2015

I think that's workable. I changed the if/else I talked about in my last comment a tiny bit to something I like the looks of more, but anyway, there we go.

I think. 😄

mantoni added a commit that referenced this pull request Mar 24, 2015
Options to not close browser on finish and/or on error.
@mantoni mantoni merged commit 05458c8 into mantoni:master Mar 24, 2015
@mantoni
Copy link
Owner

mantoni commented Mar 24, 2015

Thanks 👍

@mantoni
Copy link
Owner

mantoni commented Mar 24, 2015

Released in v2.3.0. Happy testing!

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.

2 participants