Skip to content

Implementing HTTPCookieStorage.description #1313

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 24, 2017
Merged

Implementing HTTPCookieStorage.description #1313

merged 3 commits into from
Nov 24, 2017

Conversation

vipinmenon
Copy link
Contributor

@vipinmenon vipinmenon commented Nov 10, 2017 •
edited
Loading

Implementation of the description property for HTTPCookieStorage, to remain as close as possible to its behavior on darwin.
For this program:

import Foundation

print(HTTPCookieStorage.shared)
print(HTTPCookieStorage.sharedCookieStorage(forGroupContainerIdentifier: "test"))

... the output on Darwin is:

<NSHTTPCookieStorage cookies count:12>
<NSHTTPCookieStorage cookies count:1>

With this PR the output on Linux will be

<HTTPCookieStorage cookies count:12>
<HTTPCookieStorage cookies count:1>

@ianpartridge
Copy link
Contributor

Thanks! I think we should include the NS prefix to match Darwin.

ianpartridge
ianpartridge suggested changes Nov 10, 2017
View reviewed changes
TestFoundation/TestHTTPCookieStorage.swift Outdated
@@ -109,6 +109,7 @@ class TestHTTPCookieStorage: XCTestCase {

storage.setCookie(simpleCookie)
XCTAssertEqual(storage.cookies!.count, 0)
XCTAssertEqual(storage.description, "<HTTPCookieStorage cookies count:0>")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do the new testing in a test_description() test instead?

@ianpartridge
Copy link
Contributor

@vipinmenon are you able to make the suggested changes?

@ianpartridge
Copy link
Contributor

I've made the changes to match Darwin myself, so we can get this merged. The test can be split out later.

@swift-ci please test

@ianpartridge
Copy link
Contributor

@swift-ci please test

@ianpartridge ianpartridge merged commit da93c95 into swiftlang:master Nov 24, 2017
@vipinmenon vipinmenon mentioned this pull request Dec 7, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ianpartridge ianpartridge ianpartridge requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vipinmenon @ianpartridge