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

Add asynchronous option to jscoverage_report() #227

Closed
elvis-epx opened this issue Apr 16, 2016 · 6 comments
Closed

Add asynchronous option to jscoverage_report() #227

elvis-epx opened this issue Apr 16, 2016 · 6 comments
Assignees
Milestone

Comments

@elvis-epx
Copy link

In some platforms (e.g. Raspberry) the jscoverage_report() takes so much time that it timeouts. Since a synchronous request cannot have its timeout configured, the option is to add an asynchonous option with a callback. Attached is a suggested patch that implements this feature, I have tested it in my test environment with success.

jscoverdiff.zip

@tntim96
Copy link
Owner

tntim96 commented Apr 16, 2016

Looks good, just double checking the code before committing.

@tntim96
Copy link
Owner

tntim96 commented Apr 17, 2016

Are you happy with the patch as is, or were you wanting for it to be wired into the UI? Also, I was thinking of making the arguments

function jscoverage_report(dir, callback, timeout) {
...
   request.timeout = timeout || 120000;

@elvis-epx
Copy link
Author

I'm not sure I follow, but I am happy with it, because it solves my problem. Perhaps the timeout should be longer, or configurable, but 120s is enough in Raspberry 3, which is not the typical machine.

On 17 de abr de 2016, at 05:48, tntim96 notifications@github.com wrote:

Are you happy with the patch as is, or were you wanting for it to be wired into the UI?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@tntim96 tntim96 changed the title Add asynchoronous option to jscoverage_report() Add asynchronous option to jscoverage_report() Apr 17, 2016
@tntim96
Copy link
Owner

tntim96 commented Apr 17, 2016

The fix has been committed. You can get the latest build from https://drone.io/github.com/tntim96/JSCover/files.

@tntim96
Copy link
Owner

tntim96 commented Jun 14, 2016

Just a note, I'm going to remove the deprecated synchronous call in #232.

I'm also going to change the call-back arguments to just call the request object when request.readyState == 4. Let me know if you see any potential issues with this.

@elvis-epx
Copy link
Author

No problem from my side.

Elvis

On Jun 14, 2016, at 1:10 PM, tntim96 notifications@github.com wrote:

Just a note, I'm going to remove the deprecated synchronous call in #232 #232.

I'm also going to change the call-back arguments to just call the request object when request.readyState == 4. Let me know if you see any potential issues with this.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #227 (comment), or mute the thread https://github.com/notifications/unsubscribe/AExOgADDye3QxCgah78TWpG0Y6CwJ2Nhks5qLtJygaJpZM4II_O9.

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

No branches or pull requests

2 participants