Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

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

Merged
redmunds merged 2 commits into from
Sep 14, 2013
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe3b1d0
replace node fs-extra remove() with remoteTempDirectory()
jasonsanjose Sep 13, 2013
3105bae
Use SpecRunnerUtils.deletePath() instead of removeTempDirectory
jasonsanjose Sep 13, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/spec/ExtensionInstallation-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ define(function (require, exports, module) {
afterEach(function () {
ExtensionLoader.getUserExtensionPath = realGetUserExtensionPath;
ExtensionLoader.loadExtension = realLoadExtension;
var promise = SpecRunnerUtils.remove(mockGetUserExtensionPath());
var promise = SpecRunnerUtils.removeTempDirectory();
Copy link
Copy Markdown
Contributor

@redmunds redmunds Sep 13, 2013

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member Author

@jasonsanjose jasonsanjose Sep 13, 2013

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

@redmunds redmunds Sep 13, 2013

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member Author

@jasonsanjose jasonsanjose Sep 13, 2013

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

@redmunds redmunds Sep 14, 2013

Choose a reason for hiding this comment

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

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.

waitsForDone(promise, "Mock Extension Removal", 2000);
});

Expand Down
⚡