[browser][io] Workaround for issue MoveDirectory#40310
Merged
Merged
Conversation
- Issue workaround while waiting for emscripten fix. #40305 - The following code checks for the existence of the source and destination directories which replaces the same code in the emscripten code. emscripten tracking issue: emscripten-core/emscripten#11804
Dotnet-GitSync-Bot
added
the
area-Infrastructure-coreclr
kjpou1
requested review from
MaximLipnin,
akoeplinger,
carlossanlop,
lewing,
marek-safar and
steveisok
akoeplinger
approved these changes
Aug 4, 2020
akoeplinger
left a comment
akoeplinger
left a comment
Member
There was a problem hiding this comment.
Looks good, just maybe add one comment.
…ix.cs Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
This was referenced Aug 5, 2020
Jacksondr5
pushed a commit
to Jacksondr5/runtime
that referenced
this pull request
Aug 10, 2020
* [browser][io] Workaround for issue MoveDirectory - Issue workaround while waiting for emscripten fix. dotnet#40305 - The following code checks for the existence of the source and destination directories which replaces the same code in the emscripten code. emscripten tracking issue: emscripten-core/emscripten#11804 * Update src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com> Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
mdh1418
pushed a commit
that referenced
this pull request
Aug 10, 2020
* [browser][file system] Tests System.IO.FileSystem
* Remove FileSystem from test project exclusions.
* [browser][filesystem] Comment on skipped tests due to IO.Pipes not supported
* [browser][filesystem] Comment on skipped tests due to monitor not supported
* Address review comments
* Address review comments on how to handle monitor PNSE
* Update src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
* Add ActiveIssue for browser platform
```
[ActiveIssue("#39955", TestPlatforms.Browser)]
public void NotFoundErrorIsExpected()
```
* Simplify test case with `PlatformDetection.IsSuperUser` as per review comment
* Remove unused variable from tests
* Browser platform volume does not limit each component of the path to a total of 255 characters.
- Remove the test `DirectoryWithComponentLongerThanMaxComponentAsPath_ThrowsException` for Browser platform.
- Add new test `DirectoryWithComponentLongerThanMaxComponentAsPath_BrowserDoesNotThrowsException` for Browser platform in case this check is added in the future.
* Fix windows tests
* Add ActiveIssue to test System.IO.Tests.DirectoryInfo_Name.CurrentDirectory
- System.IO.Tests.DirectoryInfo_Name.CurrentDirectory does not pass because it is using Path.GetFileName on the current directory of "/".
- See issue #39998 for more information
* Add active issue for tests that need to support file locking.
* Add active issue for tests that need to support file locking.
* Browser volume does not have a limit on segments
* Remove browser platform test
* Remove active issue for NotFoundErrorIsExpected
* Remove platform specific from SkippingHiddenFiles
* Fix enumeration test errors failing from `HiddenFilesAreReturned`.
- WebAssembly (BROWSER) has dirent d_type but is not identifying correctly but by returning UNKNOWN the managed code properly stats the file to detect if entry is directory or not.
* Fix enumeration test errors failing from `HiddenFilesAreReturned`.
- WebAssembly (BROWSER) has dirent d_type but is not identifying correctly but by returning UNKNOWN the managed code properly stats the file to detect if entry is directory or not.
* Fix build for TARGET_WASM not defined
* Address review comment by add check for IsWindows to IsSuperUser.
* Address review comment by add check for IsWindows to IsSuperUser.
* Platform Specific test for hidden files no longer needed.
* Platform Specific test fno longer needed.
* Use active issue for rename issue
* ActiveIssue no longer needed
- PR #40310 works around the issue for now while waiting for fix emscripten-core/emscripten#11804 / emscripten-core/emscripten#11812
* Use ActiveIssue for SettingUpdatesProperties
* Use ActiveIssue for TimesIncludeMillisecondPart
* Use ActiveIssue for ErrorHandlingTests failures
* Use ActiveIssue for SetUptoNanoseconds
* Use ActiveIssue for GetSetTimes test failures
* Use ActiveIssue for tests failing with DirectoryNotFoundException
* Use ActiveIssue for tests failing with UnauthorizedAccessException
* Remove debugging statement from pal_io
* Address type in CreateDirectory method name
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
ghost
locked as resolved and limited conversation to collaborators
Member
|
Did we pick up the fix now? |
Member
|
@danmosemsft thanks for the reminder #46154 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
emscripten tracking issue: emscripten-core/emscripten#11804
This will allow the issue to be resolved in the tests without waiting for emscripten release.