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

[FileStream] add tests for device and UNC paths #54545

Merged
merged 9 commits into from
Jun 24, 2021

Conversation

adamsitnik
Copy link
Member

Contributes to #54495

Does the following:

  • tests unseekable devices by opening a FileStream using handle opened from a path pointing to a named pipe (\\.\pipe\$pipeName)
  • tests seekable devices by using \\.\ and \\?\ paths that use DeviceID instead of a drive letter (sth like \\.\Volume{724edb31-eaa5-4728-a4e3-f2474fd34ae2}\Users\x\AppData\Local\Temp\y\z instead C:\Users\x\AppData\Local\Temp\y\z)
  • tests UNC paths by creating a local file share and accessing it via \\localhost\$shareName. This requires an Admin privillige and does not work on Nano, which does not provide "netapi32.dll"

These tests are going to be failing for now, we need to merge #54483 first

@adamsitnik adamsitnik added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 22, 2021
@ghost
Copy link

ghost commented Jun 22, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #54495

Does the following:

  • tests unseekable devices by opening a FileStream using handle opened from a path pointing to a named pipe (\\.\pipe\$pipeName)
  • tests seekable devices by using \\.\ and \\?\ paths that use DeviceID instead of a drive letter (sth like \\.\Volume{724edb31-eaa5-4728-a4e3-f2474fd34ae2}\Users\x\AppData\Local\Temp\y\z instead C:\Users\x\AppData\Local\Temp\y\z)
  • tests UNC paths by creating a local file share and accessing it via \\localhost\$shareName. This requires an Admin privillige and does not work on Nano, which does not provide "netapi32.dll"

These tests are going to be failing for now, we need to merge #54483 first

Author: adamsitnik
Assignees: -
Labels:

* NO MERGE *, area-System.IO

Milestone: -

@adamsitnik adamsitnik added this to the 6.0.0 milestone Jun 22, 2021
@adamsitnik adamsitnik added this to In progress in System.IO - FileStream via automation Jun 22, 2021
System.IO - FileStream automation moved this from In progress to Done Jun 22, 2021
@ViktorHofer ViktorHofer reopened this Jun 22, 2021
System.IO - FileStream automation moved this from Done to In progress Jun 22, 2021
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks for adding these. There are some compilation failures, but overall the additions LGTM.

@adamsitnik adamsitnik removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 23, 2021
@adamsitnik adamsitnik marked this pull request as ready for review June 23, 2021 07:15
@adamsitnik
Copy link
Member Author

/azp list

@adamsitnik
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik
Copy link
Member Author

The CI failures are unrelated (Android and System.ServiceProcess* failures from outerloop)

System.IO - FileStream automation moved this from In progress to Reviewer approved Jun 23, 2021
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks.

@adamsitnik adamsitnik merged commit 7833828 into dotnet:main Jun 24, 2021
System.IO - FileStream automation moved this from Reviewer approved to Done Jun 24, 2021
@adamsitnik adamsitnik deleted the devicePathTests branch June 24, 2021 06:58
thaystg added a commit to thaystg/runtime that referenced this pull request Jun 24, 2021
…bugger2

* origin/main: (107 commits)
  Disable MacCatalyst arm64 PR test runs on staging pipeline (dotnet#54678)
  [WASM] Fix async/await in config loading (dotnet#54652)
  Fix for heap_use_after_free flagged by sanitizer (dotnet#54679)
  [wasm] Bump emscripten to 2.0.23 (dotnet#53603)
  Fix compiler references when building inside VS (dotnet#54614)
  process more TLS frames at one when available (dotnet#50815)
  Add PeriodicTimer (dotnet#53899)
  UdpClient with span support (dotnet#53429)
  exclude fragile tests (dotnet#54671)
  get last error before calling a method that might fail as well (dotnet#54667)
  [FileStream] add tests for device and UNC paths (dotnet#54545)
  Fix sporadic double fd close (dotnet#54660)
  Remove Version.Clone from AssemblyName.Clone (dotnet#54621)
  [wasm] Enable fixed libraries tests (dotnet#54641)
  [wasm] Fix blazor/aot builds (dotnet#54651)
  [mono][wasm] Fix compilation error on wasm (dotnet#54659)
  Fix telemetry for Socket connects to Dns endpoints (dotnet#54071)
  [wasm] Build static components; include hot_reload in runtime (dotnet#54568)
  [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300)
  Put Crossgen2 in sync with dotnet#54235 (dotnet#54438)
  ...
@danmoseley
Copy link
Member

/backport to release/6.0-preview6

in case we want it there.

@github-actions
Copy link
Contributor

Started backporting to release/6.0-preview6: https://github.com/dotnet/runtime/actions/runs/971756061

@danmoseley
Copy link
Member

/backport to release/6.0-preview6

@github-actions
Copy link
Contributor

Started backporting to release/6.0-preview6: https://github.com/dotnet/runtime/actions/runs/972063161

@ghost ghost locked as resolved and limited conversation to collaborators Jul 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants