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

Enable NSLocale keyed archiving unit tests #596

Merged
merged 3 commits into from
Aug 25, 2016

Conversation

itaiferber
Copy link
Contributor

NSLocale's encode(with:) encodes the locale's identifier as a CFString, which causes a crash in NSKeyedArchiver's _haveVisited ref map, since CFString cannot be forcibly cast to AnyHashable. Encoding the identifier as an NSString fixes the issue.

In decoding, a different NSLocale instance is returned. Since NSLocale no longer has value semantics (which were ported to Locale), it needs an isEqual implementation to compare two NSLocale instances as equal.

These changes combined allows us to uncomment the keyed archiving test and have it pass.

@itaiferber
Copy link
Contributor Author

@swift-ci Please test

@itaiferber
Copy link
Contributor Author

The build timed out because of TestURLSession.test_downloadTaskWithURL
Going to try re-running and hoping that it won't hang this time

@itaiferber
Copy link
Contributor Author

@swift-ci Please test

@phausler phausler merged commit 80e0f05 into swiftlang:master Aug 25, 2016
@itaiferber itaiferber deleted the pr-nslocale-archiving branch August 25, 2016 23:06
itaiferber added a commit to itaiferber/swift-corelibs-foundation that referenced this pull request Aug 25, 2016
* Archive NSLocale identifier as NSString

* Override isEqual for NSLocale

* Uncomment NSLocale archiving test
itaiferber added a commit that referenced this pull request Aug 26, 2016
* Archive NSLocale identifier as NSString

* Override isEqual for NSLocale

* Uncomment NSLocale archiving test
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.

2 participants