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

Fix fake/clone tests on Windows #2778

Merged
merged 1 commit into from
Jun 11, 2019

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jun 10, 2019

Problem

Running .\build.ps1 test on Windows:

1) Error : Tests.Core.KSPManagerTests.CloneInstance_ToNotEmptyFolder_ThrowsIOException
System.IO.IOException : The process cannot access the file 'shouldntbehere.txt' because it is being used by another process.
   at System.IO.Directory.DeleteHelper(String fullPath, String userPath, Boolean recursive, Boolean throwOnTopLevelDirectoryNotFound, WIN32_FIND_DATA& data)
   at System.IO.Directory.Delete(String fullPath, String userPath, Boolean recursive, Boolean checkHost)
   at Tests.Core.KSPManagerTests.CloneInstance_ToNotEmptyFolder_ThrowsIOException() in C:\Users\Paul\github\CKAN\Tests\Core\KSPManager.cs:line 144

2) Error : Tests.Core.KSPManagerTests.FakeInstance_InNotEmptyFolder_ThrowsBadInstallLocationKraken
System.IO.IOException : The process cannot access the file 'shouldntbehere.txt' because it is being used by another process.
   at System.IO.Directory.DeleteHelper(String fullPath, String userPath, Boolean recursive, Boolean throwOnTopLevelDirectoryNotFound, WIN32_FIND_DATA& data)
   at System.IO.Directory.Delete(String fullPath, String userPath, Boolean recursive, Boolean checkHost)
   at Tests.Core.KSPManagerTests.FakeInstance_InNotEmptyFolder_ThrowsBadInstallLocationKraken() in C:\Users\Paul\github\CKAN\Tests\Core\KSPManager.cs:line 194

3) Error : Tests.Core.KSPManagerTests.FakeInstance_ValidArgumentsWithDLC_ManagerHasValidInstance
System.IO.IOException : The process cannot access the file 'registry.locked' because it is being used by another process.
   at System.IO.Directory.DeleteHelper(String fullPath, String userPath, Boolean recursive, Boolean throwOnTopLevelDirectoryNotFound, WIN32_FIND_DATA& data)
   at System.IO.Directory.Delete(String fullPath, String userPath, Boolean recursive, Boolean checkHost)
   at Tests.Core.KSPManagerTests.FakeInstance_ValidArgumentsWithDLC_ManagerHasValidInstance() in C:\Users\Paul\github\CKAN\Tests\Core\KSPManager.cs:line 214

Causes

CKAN/quotes.txt

Lines 11 to 16 in df8fb70

techman83: Hehe. :) The Linux style of locking is "here's a tiny
easy-to-miss note I'm putting on my sandwich, don't eat it!".
Windows is "I'm locking my sandwich in this safe. Why are you
worried it will go stale?"
-- pjf, IRC

Errors 1 and 2: System.IO.File.Create returns an open file handle, which on Windows includes a lock. Deleting the locked file's parent folder isn't allowed.

Error 3: KSP loads its registry.json file and creates a registry.locked file to lock it. On Windows the registry.locked file is itself locked. Deleting this locked file's parent folder also isn't allowed.

Changes

Errors 1 and 2: Now we close the file handle returned by System.IO.File.Create, which releases the lock and allows the folder to be deleted.

Error 3: Now we call RegistryManager.ReleaseLock for the KSP instance, which allows the folder to be deleted.

CloneInstance_ToNotEmptyFolder_ThrowsIOException is also renamed to CloneInstance_ToNotEmptyFolder_ThrowsPathErrorKraken to reflect the exception that is expected.

Fixes #2752.

@HebaruSan HebaruSan added Bug Something is not working as intended Pull request Windows Issues specific for Windows Tests Issues affecting the internal tests labels Jun 10, 2019
Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Nice, thanks for fixing this.
Didn't really feel like looking into Windows file handles and stuff 😅

@HebaruSan HebaruSan merged commit 8c13063 into KSP-CKAN:master Jun 11, 2019
HebaruSan added a commit that referenced this pull request Jun 11, 2019
@HebaruSan HebaruSan deleted the fix/win-fake-clone-tests branch June 11, 2019 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Tests Issues affecting the internal tests Windows Issues specific for Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fake/clone tests fail on Windows
2 participants