Skip to content

Conversation

finagolfin
Copy link
Member

In Android /tmp doesn't exist, and the temporary directory is normally
/data/local/tmp. It is not a symlink. The tests were mostly working, but
because resolvingSymlinksInPath returns a trailing slash if the
directory actually exists, in Android it was returning without the final
trailing slash in some of the cases.

The changes split the large test into smaller pieces that test one of
the behaviours of resolvingSymlinksInPath, and tries not to use the
temporarl directory by itself or suppose anything about it.

There are, however, a couple of tests that check behaviours that are
only applicable to Darwin, so those tests are only performed in Darwin
platforms.

This is simply a rebase of #2145, as @drodriguez asked me to do. This pull fixes the last remaining test that fails for me when building Foundation natively on Android.

@spevans, you already substantially reviewed this pull, let me know if there's anything else.

@spevans
Copy link
Contributor

spevans commented Jan 30, 2020

@swift-ci test

@finagolfin
Copy link
Member Author

Passed all tests, let me know if there's anything else.

@finagolfin
Copy link
Member Author

Ping, just following up.

@spevans
Copy link
Contributor

spevans commented Feb 13, 2020

@buttaface could you resolve the conflict then it should be ok to merge in

In Android /tmp doesn't exist, and the temporary directory is normally
/data/local/tmp. It is not a symlink. The tests were mostly working, but
because `resolvingSymlinksInPath` returns a trailing slash if the
directory actually exists, in Android it was returning without the final
trailing slash in some of the cases.

The changes split the large test into smaller pieces that test one of
the behaviours of `resolvingSymlinksInPath`, and tries not to use the
temporary directory by itself or suppose anything about it.

There are, however, a couple of tests that check behaviours that are
only applicable to Darwin, so those tests are only performed in Darwin
platforms.
@finagolfin
Copy link
Member Author

Done.

@spevans
Copy link
Contributor

spevans commented Feb 13, 2020

@swift-ci test linux

1 similar comment
@spevans
Copy link
Contributor

spevans commented Feb 14, 2020

@swift-ci test linux

@spevans
Copy link
Contributor

spevans commented Feb 14, 2020

@swift-ci test and merge

@finagolfin
Copy link
Member Author

Maybe you have to approve it on github first?

@spevans spevans merged commit fb1a959 into swiftlang:master Feb 14, 2020
@spevans
Copy link
Contributor

spevans commented Feb 14, 2020

@buttaface There is often issues between GitHub and the Swift CI servers whereby messages from one to the other seem to get lost. Since it passed ok on Linux I have merged it in manually.

@finagolfin
Copy link
Member Author

Alright, thanks.

@finagolfin finagolfin deleted the android-testurl branch February 14, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants