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

[SR-7938] Add URLProtectionSpace.description #1607

Merged
merged 7 commits into from Jun 21, 2018

Conversation

cmilr
Copy link
Contributor

@cmilr cmilr commented Jun 16, 2018

Hi, first-time contributor here—hoping this PR is useful. It was actually harder to implement than I was expecting.

// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2016 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a new file so that date should just be 2018

@spevans
Copy link
Collaborator

spevans commented Jun 17, 2018

Could you also add the TestURLProtectionSpace.swift file into the DarwinCompatibilityTests Xcode project that is in the swift-corelibs-foundation directory. This allows running the tests against Darwin's native foundation library. Don't worry that all of the tests don't pass just that the new tests you wrote do.

@spevans
Copy link
Collaborator

spevans commented Jun 17, 2018

You may need to change XCTAssert(space.description.hasPrefix("<URLProtectionSpace ")) to XCTAssert(space.description.hasPrefix("<\(type(of: space)) ")) since Darwin looks to use NSURLProtectionSpace internally

@spevans
Copy link
Collaborator

spevans commented Jun 17, 2018

@swift-ci please test

@cmilr
Copy link
Contributor Author

cmilr commented Jun 18, 2018

@spevans in DarwinCompatibilityTests I had to comment out a couple of tests in TestBundle, as they were causing the project not to build. Not sure if I should leave them commented out, or how that works? Otherwise, I got my code passing with one minor tweak to my test file. Now it passes in both.

@parkera
Copy link
Member

parkera commented Jun 20, 2018

Welcome @cmilr and thanks for the contribution!

Copy link
Member

@parkera parkera left a comment

Choose a reason for hiding this comment

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

LGTM

@millenomi
Copy link
Contributor

@swift-ci please test and merge

1 similar comment
@spevans
Copy link
Collaborator

spevans commented Jun 21, 2018

@swift-ci please test and merge

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.

None yet

5 participants