gruntjs / grunt-lib-phantomjs

Grunt and PhantomJS, sitting in a tree.
http://gruntjs.com/
MIT License
93 stars 63 forks source link

Unlink temp file only when we are done with it #88

Closed Louisblack closed 9 years ago

Louisblack commented 9 years ago

Windows doesn't like attempting to unlink a file when it is in use. This change ensures we only attempt to unlink once we are done. I've tested it on a project that was consistently having the issue and it fixed it. This change removes the need for a possible infinitely looping setTimeout as suggested in the other pull request for this issue.

Xeio commented 9 years ago

This change removes the need for a possible infinitely looping setTimeout as suggested in the other pull request for this issue.

If you're talking about #85 there isn't any infinite loop there.

Also, this won't fix the issue. The unlink only might work immediately after the kill. It may require at least one setTimeout(0, ...) for Windows to release the file.

Louisblack commented 9 years ago

I've tested it with a project that was consistently having the unlink error. This change has removed the issue for me. In what situation do you think the setTimeout will be required? I'm happy to try and reproduce it with the proposed change.

To be honest, at the moment I'd be happy with any change that fixes this because I have a few CI builds that are consistently failing because of this issue

Xeio commented 9 years ago

I have no idea what causes it, but I copied your change into my local repo and can still reproduce.

The project I'm reproducing with is TSWCalc for reference.

Otherwise I'm not really sure what's required for Windows to behave like this:

Windows 10 SSD Intel processor

Louisblack commented 9 years ago

I've just cloned that project and you're right. It's not fixed.

My projects are using grunt-contrib-jasmine to run tests which seems to work with my change. Maybe it's something to do with how these two different testing modules interact with phantom.js.

Curses!