-
Notifications
You must be signed in to change notification settings - Fork 381
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
Fix PR inconsistent test failure #8094
Conversation
…tead just not try again.
Fixes #8072 |
Hmm according to the test failures in this PR the change didn't help. |
It did help! However, it unveiled the actual reason this hack was there:
Doing some searching, I found the exact place this hack was copied from: https://stackoverflow.com/a/47398010/294804 //delete the directory and it's contents if the directory exists
if (Directory.Exists(dirPath)) {
try {
Directory.Delete(dirPath, recursive: true); //throws if directory doesn't exist.
} catch {
//HACK because the recursive delete will throw with an "Directory is not empty."
//exception after it deletes all the contents of the diretory if the directory
//is open in the left nav of Windows's explorer tree. This appears to be a caching
//or queuing latency issue. Waiting 2 secs for the recursive delete of the directory's
//contents to take effect solved the issue for me. Hate it I do, but it was the only
//way I found to work around the issue.
Thread.Sleep(2000); //wait 2 seconds
Directory.Delete(dirPath, recursive: true);
}
} So, the hack was trying to avoid this recursive deletion issue. Let me see if I can find a better solution and update the PR. |
…of the hack to begin with.
After looking over this thread, I've determined the only 2 exception types I need to care about are |
Summary
After looking over many PRs recently, I noticed that they inconsistently fail. However, no tests are failing (all tests pass). If you look over the test log, you'll see an exception at the very bottom:
Looking into this, there was a "hack" where if the temporary test folder could not be deleted, it slept for 2 seconds and then tried again. This second attempt is what was failing. This only seems to happen on the last test execution in the TemplatePackageManagerTests. My theory is the test framework is shutting down and, somehow, this Dispose runs after the point where it can still access this folder. So, it just fails.
Either way, since these folders are created in the Temp machine folder, it really doesn't matter if we cannot clean up the last test folder. I'd rather the PR runs not randomly fail. I've also made the catch specifically catch this UnauthorizedAccessException instead of everything. Other exceptions are not expected.