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

test: properly clean up temp directory #2164

Closed
wants to merge 1 commit into from

Conversation

silverwind
Copy link
Contributor

This test didn't clean up its temp directory when the mkdirSync call fails, which broke future runs of this test with EEXIST errors. This change ensures the temp directory is always cleaned up.

The reason I encountered this is because it seems to be a persistent failure on OS X 10.11, at least on my machine.

@silverwind silverwind added the test Issues and PRs related to the tests. label Jul 11, 2015
throw e;
}
}

process.chdir(dir);
assert(process.cwd() == dir);

process.chdir('..');
assert(process.cwd() == path.resolve(common.fixturesDir));
Copy link
Contributor

Choose a reason for hiding this comment

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

I have actually consistently seen this assertion fail on 10.11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what I was seeing to. Thanks for confirming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, it fails on assert(process.cwd() == dir); already right now.

@evanlucas
Copy link
Contributor

LGTM if CI is happy

@silverwind
Copy link
Contributor Author

silverwind added a commit that referenced this pull request Jul 11, 2015
A persistent failure on OS X 10.11 uncovered a inproperly cleaned up
temp directory in this test. This changes the mkdirSync call to clean up
properly in case it throws.

PR-URL: #2164
Reviewed-By: Evan Lucas <[email protected]>
@silverwind
Copy link
Contributor Author

Lots of unrelated failures in the CI. Landed in d4ceb16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants