Add ability to create temp mapped drive for integration tests #8366
Conversation
| // let's create the mapped drive only once it's needed by any test, then let's reuse; | ||
| _mappedDrive ??= new DummyMappedDrive(); | ||
| unevaluatedInclude = UpdatePathToMappedDrive(unevaluatedInclude, _mappedDrive.MappedDriveLetter); |
There was a problem hiding this comment.
nit: just wondering if we can request the specific drive instead, imo this makes it a bit more readable
using DummyMappedDrive mappedDrive = DriveMapping.GetDrive("z");
// test content
There was a problem hiding this comment.
The drive to request is unknown upfront - it needs to be untaken on the system.
So the _mappedDrive takes care about determining one that is free and using it.
Added DriveMapping is utility class that can be used for that purpose.
tl;dr;: Explicit assigning is intentionally hidden in this wrapper.
There was a problem hiding this comment.
in this case, using placeholder in test data might be more readable, and then it is replaced by DriveMapping.
e.g: "%DRIVE%:\**\*.cs"
imo, for test purposes picking up 'z' or other letter at the end of abc should work.
There was a problem hiding this comment.
I like the %DRIVE%: suggestion! - updated.
I wouldn't use any fixed drive letter though (ppl tend to map shares to various drive letters). The dynamic drive letter allocation logic isn't complicated - so 'd keep it.
| { | ||
| path = driveLetter + path.Substring(1); | ||
| } | ||
| return path; |
There was a problem hiding this comment.
If path is empty, should we return "" or driveLetter + ":"? I'm curious if this could artificially make the drive enumeration tests pass.
There was a problem hiding this comment.
Empty paths are valid test case scenarios (e.g. unspecified exclude pattern). So it intentionaly leaves unspecified or unrooted paths unaffected
|
We should also fix the Unix version of these tests.
|
|
Added for unix long run and skipping for now: #8373 |
JanKrivanek
added
the
merge-when-branch-open
Fixes #7330 (plus one subtask of #8329) Changes Made Based on Add ability to create temp mapped drive for integration tests #8366 fixes to enable other Drive enumeration integration tests with a dummy folder in windows Remove one test data https://github.com/dotnet/msbuild/blob/fecef0fdffe59ba8b0251701a23be48bbd552726/src/Build.OM.UnitTests/Instance/ProjectItemInstance_Tests.cs#L1010-L1012C45 since there is no warning when inlude is not null and exclude with enumerating wildcards. The related logical code is msbuild/src/Build/Utilities/EngineFileUtilities.cs Line 339 in fecef0f private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLoggingContext targetLoggingContext, IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, bool disableExcludeDriveEnumerationWarning, string fileSpec) . There is no condition satisfied. Associate unix Enumeration Tests long time run with issue Unix drive enumeration imports not expanded? #8373
Fixes dotnet#7330 (plus one subtask of dotnet#8329) Changes Made Based on Add ability to create temp mapped drive for integration tests dotnet#8366 fixes to enable other Drive enumeration integration tests with a dummy folder in windows Remove one test data https://github.com/dotnet/msbuild/blob/fecef0fdffe59ba8b0251701a23be48bbd552726/src/Build.OM.UnitTests/Instance/ProjectItemInstance_Tests.cs#L1010-L1012C45 since there is no warning when inlude is not null and exclude with enumerating wildcards. The related logical code is msbuild/src/Build/Utilities/EngineFileUtilities.cs Line 339 in fecef0f private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLoggingContext targetLoggingContext, IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, bool disableExcludeDriveEnumerationWarning, string fileSpec) . There is no condition satisfied. Associate unix Enumeration Tests long time run with issue Unix drive enumeration imports not expanded? dotnet#8373
Fixes #7330 (plus one subtask of #8329) Changes Made Based on Add ability to create temp mapped drive for integration tests #8366 fixes to enable other Drive enumeration integration tests with a dummy folder in windows Remove one test data https://github.com/dotnet/msbuild/blob/fecef0fdffe59ba8b0251701a23be48bbd552726/src/Build.OM.UnitTests/Instance/ProjectItemInstance_Tests.cs#L1010-L1012C45 since there is no warning when inlude is not null and exclude with enumerating wildcards. The related logical code is msbuild/src/Build/Utilities/EngineFileUtilities.cs Line 339 in fecef0f private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLoggingContext targetLoggingContext, IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, bool disableExcludeDriveEnumerationWarning, string fileSpec) . There is no condition satisfied. Associate unix Enumeration Tests long time run with issue Unix drive enumeration imports not expanded? #8373
Fixes #7330
(plus one subtask of #8329)
Tests only change (no production code affected)
Context
Drive enumeration integration tests need to simulate attempt to enumerate whole drive.
To prevent security and test runtime considerations - a dummy folder is created and mapped to a free letter to be offered to test as a drive for enumeration.
Changes Made
Added utility for mapping drives and mounted to affected unit tests.