replace node fs-extra remove() with removeTempDirectory()

[jasonsanjose]

Fix intermittent failure (see 0.31.0.-9452 on windows) by replacing fs-extra.remove() in Node with brackets.fs.unlink via SpecRunnerUtils.removeTempDirectory(). Possible related issue from fs-extra rim-raf dependency isaacs/rimraf#19.

Error: Expected { errno : 53, code : 'ENOTEMPTY', path : 'C:\ws\Brackets-git\brackets\test\temp\extensions\user\basic-valid-extension' } to be null.
    at new jasmine.ExpectationResult (file:///C:/ws/Brackets-git/brackets/test/thirdparty/jasmine-core/jasmine.js:102:32)
    at null.toBeNull (file:///C:/ws/Brackets-git/brackets/test/thirdparty/jasmine-core/jasmine.js:1194:29)
    at NodeConnection.<anonymous> (file:///C:/ws/Brackets-git/brackets/test/spec/ExtensionInstallation-test.js:67:29)
    at NodeConnection.<anonymous> (file:///C:/ws/Brackets-git/brackets/src/thirdparty/jquery-2.0.1.min.js:4:26181)
    at l (file:///C:/ws/Brackets-git/brackets/src/thirdparty/jquery-2.0.1.min.js:4:24797)
    at Object.c.fireWith (file:///C:/ws/Brackets-git/brackets/src/thirdparty/jquery-2.0.1.min.js:4:25618)
    at NodeConnection._receive (file:///C:/ws/Brackets-git/brackets/src/utils/NodeConnection.js:422:34)

[redmunds]

Do you think deleting the entire temp folder after every spec could cause problems? I know that the LowLevelFileIO tests do this, but now (for performance) we keep Brackets window open across multiple specs, so that seems like it could lead to using a temp file across multiple specs.

Maybe removeTempDirectory() should take a subfolder parameter so it could append this to temp folder path and only delete a subfolder inside the temp folder.

[jasonsanjose]

It shouldn't cause problems. Each suite should be cleaning up after itself and not have suite-to-suite dependencies like that anyhow.

[redmunds]

SpecRunner doesn't wait until 1 suite is done before starting the next suite -- the specs are intermingled -- so 1 suite can delete files being used by another suite.

[jasonsanjose]

Are you sure about that? That would seem to violate the expectations of beforeEach/afterEach. I don't think I've ever seen that behavior.

Regardless, I understand your concern about the overreach. I'll use deletePath instead...still goes through brackets.fs instead of node. Fix pushed.

[redmunds]

SpecRunner doesn't wait until 1 suite is done before starting the next suite -- the specs are intermingled

You're right -- what I said does not seem to be true. SpecRunner always seemed like it was running things in any order, but watching it more closely, it seems to run only 1 suite at a time.

[redmunds]

Done with review. Just a question about possibly deleting too much.

[jasonsanjose]

Actually...hold on. I might have found an issue.

[jasonsanjose]

@redmunds false alarm. All tests recreate the temp folder as they need it via SpecRunner.copyPath. We're good.

[redmunds]

Looks good. Merging.