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

Resolve tvOS/iOS FileSystem test failures #74927

Merged
merged 9 commits into from
Oct 26, 2022
Merged

Conversation

directhex
Copy link
Contributor

Use SetCurrentDirectory on tests which use an unrooted path on non-Windows out of the app's working directory, which is readonly on some platforms.

Closes: #67853

non-Windows out of the app's working directory, which is readonly on
some platforms.

Closes: dotnet#67853
@ghost
Copy link

ghost commented Sep 1, 2022

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

Issue Details

Use SetCurrentDirectory on tests which use an unrooted path on non-Windows out of the app's working directory, which is readonly on some platforms.

Closes: #67853

Author: directhex
Assignees: directhex
Labels:

area-System.IO

Milestone: -

@dotnet dotnet deleted a comment from azure-pipelines bot Sep 1, 2022
@dotnet dotnet deleted a comment from azure-pipelines bot Sep 1, 2022
@dotnet dotnet deleted a comment from azure-pipelines bot Sep 1, 2022
@dotnet dotnet deleted a comment from azure-pipelines bot Sep 7, 2022
@directhex directhex marked this pull request as ready for review September 7, 2022 22:24
@directhex
Copy link
Contributor Author

Outstanding failures are unrelated infra, I think

@dotnet dotnet deleted a comment from azure-pipelines bot Sep 9, 2022
@dotnet dotnet deleted a comment from azure-pipelines bot Sep 9, 2022
@dotnet dotnet deleted a comment from azure-pipelines bot Sep 12, 2022
@dotnet dotnet deleted a comment from azure-pipelines bot Sep 16, 2022
@directhex
Copy link
Contributor Author

/azp run runtime-extra-platforms

@dotnet dotnet deleted a comment from azure-pipelines bot Sep 27, 2022
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

I see that you're wrapping some of those tests with a remote executor and adding the SetCurrentDirectory invocation at the beginning. I'd like to assume it's fine to do this in a remote executor context, because I worry that changing the current directory would affect the current process. But as long as that doesn't happen, we're ok.

I am also seeing that you're excluding tvos/ios/browser from the test platforms. I guess that is fine, because IIRC, remote executor does not work in any of those platforms.

@adamsitnik @jozkee I'm approving it but if you can take an extra look, that would be great.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @directhex !

@adamsitnik adamsitnik merged commit 193ee83 into dotnet:main Oct 26, 2022
@adamsitnik adamsitnik added this to the 8.0.0 milestone Oct 26, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 25, 2022
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.

[tests][tvOS] Some System.IO tests are failing
3 participants