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

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

Merged
merged 3 commits into from
Jul 14, 2020

Conversation

akoeplinger
Copy link
Member

Some of the VB tests need culture data.

For DriveInfo we provide an implementation for Browser that returns stub values.

@akoeplinger akoeplinger force-pushed the microsoft-vb-driveinfo branch from 30bf384 to 080e22c Compare July 14, 2020 16:02
public sealed partial class DriveInfo
{
public DriveType DriveType => DriveType.Unknown;
public string DriveFormat => "memfs";
Copy link
Member

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

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


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

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

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 :)

@@ -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

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

Choose a reason for hiding this comment

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

Yes.

@akoeplinger akoeplinger merged commit d9b7f4e into dotnet:master Jul 14, 2020
@akoeplinger akoeplinger deleted the microsoft-vb-driveinfo branch July 14, 2020 20:40
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants