Skip to content

WASM: Fix System.IO.FileSystem.DriveInfo and Microsoft.VisualBasic.Core tests #39276

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.

Sign up for GitHub

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

Merged
akoeplinger merged 3 commits into from
Jul 14, 2020
Merged

WASM: Fix System.IO.FileSystem.DriveInfo and Microsoft.VisualBasic.Core tests #39276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae94a58
WASM: Fix System.IO.FileSystem.DriveInfo
akoeplinger Jul 14, 2020
080e22c
WASM: Fix Microsoft.VisualBasic.Core tests
akoeplinger Jul 14, 2020
5a97727
Fix test that was accidentally reversed for non-Browser
akoeplinger Jul 14, 2020
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 src/libraries/Microsoft.VisualBasic.Core/tests/FinancialTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class FinancialTests

// The accuracy to which we can validate some numeric test cases depends on the platform.
private static readonly int s_precision = IsArmOrArm64OrAlpine ? 12 :
PlatformDetection.IsNetFramework ? 14 : 15;
(PlatformDetection.IsBrowser || PlatformDetection.IsNetFramework) ? 14 : 15;
Copy link
Member

@danmoseley danmoseley Jul 14, 2020

Choose a reason for hiding this comment

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

Just curious, this is due to a limitation in the libc that we use in browser?

Copy link
Member Author

@akoeplinger akoeplinger Jul 14, 2020

Choose a reason for hiding this comment

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

Yes.


[Theory]
[InlineData(0, 1.0, 1.0, 1.0, 1.0, 0, 0)]
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Microsoft.VisualBasic.Core/tests/ObjectTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ public static IEnumerable<object[]> ObjTst_TestData()
yield return new object[] { "a", "a", 0, 0 };
yield return new object[] { "a", "b", -1, -1 };
yield return new object[] { "b", "a", 1, 1 };
yield return new object[] { "a", "ABC", 32, -1 };
yield return new object[] { "ABC", "a", -32, 1 };
yield return new object[] { "a", "ABC", 32, PlatformDetection.IsInvariantGlobalization ? -2 : -1 };
yield return new object[] { "ABC", "a", -32, PlatformDetection.IsInvariantGlobalization ? 2 : 1 };
yield return new object[] { "abc", "ABC", 32, 0 };
}
}
Expand Down
31 changes: 18 additions & 13 deletions src/libraries/Microsoft.VisualBasic.Core/tests/StringTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,25 +364,30 @@ public void MidStmtStr_ArgumentException(string str, int start, int length, stri
}

[Theory]
[InlineData(null, null, 0, 0)]
[InlineData(null, "", 0, 0)]
[InlineData("", null, 0, 0)]
[InlineData(null, "a", -1, -1)]
[InlineData("a", null, 1, 1)]
[InlineData("", "a", -97, -1)]
[InlineData("a", "", 97, 1)]
[InlineData("a", "a", 0, 0)]
[InlineData("a", "b", -1, -1)]
[InlineData("b", "a", 1, 1)]
[InlineData("a", "ABC", 32, -1)]
[InlineData("ABC", "a", -32, 1)]
[InlineData("abc", "ABC", 32, 0)]
[MemberData(nameof(StrCmp_TestData))]
public void StrCmp(string left, string right, int expectedBinaryCompare, int expectedTextCompare)
{
Assert.Equal(expectedBinaryCompare, StringType.StrCmp(left, right, TextCompare: false));
Assert.Equal(expectedTextCompare, StringType.StrCmp(left, right, TextCompare: true));
}

public static IEnumerable<object[]> StrCmp_TestData()
{
yield return new object[] { null, null, 0, 0 };
yield return new object[] { null, "", 0, 0 };
yield return new object[] { "", null, 0, 0 };
yield return new object[] { null, "a", -1, -1 };
yield return new object[] { "a", null, 1, 1 };
yield return new object[] { "", "a", -97, -1 };
yield return new object[] { "a", "", 97, 1 };
yield return new object[] { "a", "a", 0, 0 };
yield return new object[] { "a", "b", -1, -1 };
yield return new object[] { "b", "a", 1, 1 };
yield return new object[] { "a", "ABC", 32, PlatformDetection.IsInvariantGlobalization ? -2 : -1 };
yield return new object[] { "ABC", "a", -32, PlatformDetection.IsInvariantGlobalization ? 2 : 1 };
yield return new object[] { "abc", "ABC", 32, 0 };
}

[Theory]
[InlineData(null, null, true, true)]
[InlineData("", null, true, true)]
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Microsoft.VisualBasic.Core/tests/StringsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void Asc_Chr_Invariant(int charCode, int expected)
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/30419", TargetFrameworkMonikers.NetFramework)]
[Theory]
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInvariantGlobalization))]
[InlineData(0, 0)]
[InlineData(33, 33)]
[InlineData(172, 0)]
Expand Down
7 changes: 6 additions & 1 deletion src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@
<Compile Include="$(CommonPath)System\IO\PathInternal.Windows.cs"
Link="Common\System\IO\PathInternal.Windows.cs" />
</ItemGroup>
<ItemGroup Condition="('$(TargetsUnix)' == 'true' or '$(TargetsBrowser)' == 'true') And '$(TargetFramework)' == '$(NetCoreAppCurrent)' ">
<ItemGroup Condition="'$(TargetsUnix)' == 'true' And '$(TargetFramework)' == '$(NetCoreAppCurrent)' ">
<Compile Include="System\IO\DriveInfo.UnixOrBrowser.cs" />
<Compile Include="System\IO\DriveInfo.Unix.cs" />
<Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs"
Link="Common\Interop\Unix\Interop.Libraries.cs" />
Expand All @@ -66,6 +67,10 @@
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MountPoints.FormatInfo.cs"
Link="Common\Interop\Unix\Interop.MountPoints.FormatInfo.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsBrowser)' == 'true' And '$(TargetFramework)' == '$(NetCoreAppCurrent)' ">
<Compile Include="System\IO\DriveInfo.UnixOrBrowser.cs" />
<Compile Include="System\IO\DriveInfo.Browser.cs" />
</ItemGroup>
<ItemGroup>
<Reference Include="System.IO.FileSystem" />
<Reference Include="System.Memory" />
Expand Down
20 changes: 20 additions & 0 deletions src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Browser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Security;

namespace System.IO
{
public sealed partial class DriveInfo
{
public DriveType DriveType => DriveType.Unknown;
public string DriveFormat => "memfs";
Copy link
Member

@danmoseley danmoseley Jul 14, 2020

Choose a reason for hiding this comment

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

I see "memfs" is a node.js plugin. Is this the value that I would get in the browser context, where node.js is not present - would that be confusing?

Copy link
Member Author

@akoeplinger akoeplinger Jul 14, 2020

Choose a reason for hiding this comment

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

memfs is the default Emscripten file system that we're using to emulate an FS in the browser: https://emscripten.org/docs/api_reference/Filesystem-API.html

public long AvailableFreeSpace => 0;
public long TotalFreeSpace => 0;
public long TotalSize => 0;

private static string[] GetMountPoints() => Environment.GetLogicalDrives();
}
}
40 changes: 2 additions & 38 deletions src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,6 @@ namespace System.IO
{
public sealed partial class DriveInfo
{
public static DriveInfo[] GetDrives()
{
string[] mountPoints = Interop.Sys.GetAllMountPoints();
DriveInfo[] info = new DriveInfo[mountPoints.Length];
for (int i = 0; i < info.Length; i++)
{
info[i] = new DriveInfo(mountPoints[i]);
}

return info;
}

private static string NormalizeDriveName(string driveName)
{
if (driveName.Contains("\0")) // string.Contains(char) is .NetCore2.1+ specific
{
throw new ArgumentException(SR.Format(SR.Arg_InvalidDriveChars, driveName), nameof(driveName));
}
if (driveName.Length == 0)
{
throw new ArgumentException(SR.Arg_MustBeNonEmptyDriveName, nameof(driveName));
}
return driveName;
}

public DriveType DriveType
{
get
Expand Down Expand Up @@ -104,19 +79,6 @@ public long TotalSize
}
}

[AllowNull]
public string VolumeLabel
{
get
{
return Name;
}
set
{
throw new PlatformNotSupportedException();
}
}

private void CheckStatfsResultAndThrowIfNecessary(int result)
{
if (result != 0)
Expand All @@ -132,5 +94,7 @@ private void CheckStatfsResultAndThrowIfNecessary(int result)
}
}
}

private static string[] GetMountPoints() => Interop.Sys.GetAllMountPoints();
}
}
50 changes: 50 additions & 0 deletions src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.UnixOrBrowser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Security;

namespace System.IO
{
public sealed partial class DriveInfo
{
public static DriveInfo[] GetDrives()
{
string[] mountPoints = GetMountPoints();
DriveInfo[] info = new DriveInfo[mountPoints.Length];
for (int i = 0; i < info.Length; i++)
{
info[i] = new DriveInfo(mountPoints[i]);
}

return info;
}

private static string NormalizeDriveName(string driveName)
{
if (driveName.Contains("\0")) // string.Contains(char) is .NetCore2.1+ specific
Copy link
Member

@danmoseley danmoseley Jul 14, 2020

Choose a reason for hiding this comment

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

Incidentally I think this comment is out of date and if (driveName.Contains('\0')) ought to compile.

Copy link
Member Author

@akoeplinger akoeplinger Jul 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah looks like it is outdated but since this is preexisting code I'll leave the cleanup to later on :)

{
throw new ArgumentException(SR.Format(SR.Arg_InvalidDriveChars, driveName), nameof(driveName));
}
if (driveName.Length == 0)
{
throw new ArgumentException(SR.Arg_MustBeNonEmptyDriveName, nameof(driveName));
}
return driveName;
}

[AllowNull]
public string VolumeLabel
{
get
{
return Name;
}
set
{
throw new PlatformNotSupportedException();
}
}
}
}
20 changes: 15 additions & 5 deletions src/libraries/System.IO.FileSystem.DriveInfo/tests/DriveInfo.Unix.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void TestGetDrives()
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix)]
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
public void PropertiesOfInvalidDrive()
{
string invalidDriveName = "NonExistentDriveName";
Expand All @@ -64,16 +64,26 @@ public void PropertiesOfInvalidDrive()
public void PropertiesOfValidDrive()
{
var root = new DriveInfo("/");
Assert.True(root.AvailableFreeSpace > 0);
var format = root.DriveFormat;
Assert.Equal(DriveType.Fixed, root.DriveType);
Assert.Equal(PlatformDetection.IsBrowser ? DriveType.Unknown : DriveType.Fixed, root.DriveType);
Assert.True(root.IsReady);
Assert.Equal("/", root.Name);
Assert.Equal("/", root.ToString());
Assert.Equal("/", root.RootDirectory.FullName);
Assert.True(root.TotalFreeSpace > 0);
Assert.True(root.TotalSize > 0);
Assert.Equal("/", root.VolumeLabel);

if (PlatformDetection.IsBrowser)
{
Assert.True(root.AvailableFreeSpace == 0);
Assert.True(root.TotalFreeSpace == 0);
Assert.True(root.TotalSize == 0);
}
else
{
Assert.True(root.AvailableFreeSpace > 0);
Assert.True(root.TotalFreeSpace > 0);
Assert.True(root.TotalSize > 0);
}
}

[Fact]
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/tests.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

<ItemGroup Condition="'$(TargetOS)' == 'Browser' and '$(RunDisabledWasmTests)' != 'true'">
<!-- Builds currently do not pass -->
<ProjectExclusions Include="$(MSBuildThisFileDirectory)Microsoft.VisualBasic.Core\tests\Microsoft.VisualBasic.Core.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.CodeDom\tests\System.CodeDom.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.ComponentModel.Primitives\tests\System.ComponentModel.Primitives.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.ComponentModel.TypeConverter\tests\System.ComponentModel.TypeConverter.Tests.csproj" />
Expand All @@ -34,7 +33,6 @@
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Globalization.Calendars\tests\System.Globalization.Calendars.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Globalization.Extensions\tests\System.Globalization.Extensions.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Globalization\tests\System.Globalization.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.FileSystem.DriveInfo\tests\System.IO.FileSystem.DriveInfo.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.FileSystem\tests\System.IO.FileSystem.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.MemoryMappedFiles\tests\System.IO.MemoryMappedFiles.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.Packaging\tests\System.IO.Packaging.Tests.csproj" />
Expand Down
1 change: 0 additions & 1 deletion src/mono/wasm/wasm.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.Runtime\$(NetCoreAppCurrent)-$(Configuration)\System.Runtime.dll"/>
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.Console\$(NetCoreAppCurrent)-Browser-$(Configuration)\System.Console.dll"/>
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.IO.FileSystem\$(NetCoreAppCurrent)-Browser-$(Configuration)\System.IO.FileSystem.dll"/>
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.IO.FileSystem.DriveInfo\$(NetCoreAppCurrent)-Browser-$(Configuration)\System.IO.FileSystem.DriveInfo.dll"/>
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.IO.MemoryMappedFiles\$(NetCoreAppCurrent)-Browser-$(Configuration)\System.IO.MemoryMappedFiles.dll"/>
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.Net.Sockets\$(NetCoreAppCurrent)-Browser-$(Configuration)\System.Net.Sockets.dll"/>
<WasmPInvokeAssemblies Include="$(ArtifactsBinDir)\System.Net.Primitives\$(NetCoreAppCurrent)-Browser-$(Configuration)\System.Net.Primitives.dll"/>
Expand Down