From 341872b50ebda1d0dce378bbd636f70000fe70bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez=20Troiti=C3=B1o?= Date: Mon, 30 Sep 2019 15:03:35 -0700 Subject: [PATCH] [test] Rewrite URLSession cookie tests to be independent. Some cookie tests were depending on previously run tests to be successful. Rewrite all the tests to be independent from each other. The first change is that all the tests that deal with cookies try to clean their cookie storages before starting the testing. This should avoid previous state leaking into the current test. Some other changes are made to clarify how the cookie tests are actually performed. To check that the cookies are set by URLSession, an special endpoint that echoes back the request header are used. To make it clear, that endpoint has changed names. In the cases that we were checking that the response headers were setting the cookies, we actually check for the right headers to been set, not some random data in the response (because for some reason, the request headers were also being echo in another endpoint, which might have created false positives). There is also some cleaning of unused variables and var that should have been let. I tested this by running all the tests in TestURLSession in the same run, and also running the individual tests one by one from the command line. --- TestFoundation/HTTPServer.swift | 9 +- TestFoundation/TestURLSession.swift | 164 +++++++++++++++++++--------- 2 files changed, 116 insertions(+), 57 deletions(-) diff --git a/TestFoundation/HTTPServer.swift b/TestFoundation/HTTPServer.swift index 0314a9c609..96d3d5c028 100644 --- a/TestFoundation/HTTPServer.swift +++ b/TestFoundation/HTTPServer.swift @@ -588,17 +588,16 @@ public class TestURLSessionServer { } if uri == "/requestCookies" { - let text = request.getCommaSeparatedHeaders() - return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)\r\nSet-Cookie: fr=anjd&232; Max-Age=7776000; path=/\r\nSet-Cookie: nm=sddf&232; Max-Age=7776000; path=/; domain=.swift.org; secure; httponly\r\n", body: text) + return _HTTPResponse(response: .OK, headers: "Set-Cookie: fr=anjd&232; Max-Age=7776000; path=/\r\nSet-Cookie: nm=sddf&232; Max-Age=7776000; path=/; domain=.swift.org; secure; httponly\r\n", body: "") } - if uri == "/setCookies" { + if uri == "/echoHeaders" { let text = request.getCommaSeparatedHeaders() return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)", body: text) } - if uri == "/redirectSetCookies" { - return _HTTPResponse(response: .REDIRECT, headers: "Location: /setCookies\r\nSet-Cookie: redirect=true; Max-Age=7776000; path=/", body: "") + if uri == "/redirectToEchoHeaders" { + return _HTTPResponse(response: .REDIRECT, headers: "Location: /echoHeaders\r\nSet-Cookie: redirect=true; Max-Age=7776000; path=/", body: "") } if uri == "/UnitedStates" { diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index f5ff28d2ae..2f8282233d 100644 --- a/TestFoundation/TestURLSession.swift +++ b/TestFoundation/TestURLSession.swift @@ -283,7 +283,7 @@ class TestURLSession : LoopbackServerTest { let headers = ["header1": "value1"] req.httpMethod = "POST" req.allHTTPHeaderFields = headers - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") @@ -310,7 +310,7 @@ class TestURLSession : LoopbackServerTest { let headers = ["header1": "rvalue1", "header2": "rvalue2", "Header4": "rvalue4"] req.httpMethod = "POST" req.allHTTPHeaderFields = headers - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") @@ -334,7 +334,7 @@ class TestURLSession : LoopbackServerTest { let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) var expect = expectation(description: "GET \(urlString): no timeout") let req = URLRequest(url: URL(string: urlString)!) - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } XCTAssertNil(error as? URLError, "error = \(error as! URLError)") } @@ -351,7 +351,7 @@ class TestURLSession : LoopbackServerTest { var expect = expectation(description: "GET \(urlString): will timeout") var req = URLRequest(url: URL(string: "http://127.0.0.1:-1/Peru")!) req.timeoutInterval = 1 - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } XCTAssertNotNil(error) } @@ -414,7 +414,6 @@ class TestURLSession : LoopbackServerTest { config.timeoutIntervalForRequest = 8 let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) let expect = expectation(description: "GET \(urlString): simple HTTP/0.9 response") - var expectedResult = "unknown" let task = session.dataTask(with: url) { data, response, error in XCTAssertNotNil(data) XCTAssertNotNil(response) @@ -564,25 +563,34 @@ class TestURLSession : LoopbackServerTest { } } - func test_disableCookiesStorage() { - let config = URLSessionConfiguration.default - config.timeoutIntervalForRequest = 5 - config.httpCookieAcceptPolicy = HTTPCookie.AcceptPolicy.never - if let storage = config.httpCookieStorage, let cookies = storage.cookies { + func emptyCookieStorage(storage: HTTPCookieStorage?) { + if let storage = storage, let cookies = storage.cookies { for cookie in cookies { storage.deleteCookie(cookie) } } + } + + func test_disableCookiesStorage() { + let config = URLSessionConfiguration.default + config.timeoutIntervalForRequest = 5 + config.httpCookieAcceptPolicy = HTTPCookie.AcceptPolicy.never + emptyCookieStorage(storage: config.httpCookieStorage) XCTAssertEqual(config.httpCookieStorage?.cookies?.count, 0) let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) var expect = expectation(description: "POST \(urlString)") var req = URLRequest(url: URL(string: urlString)!) req.httpMethod = "POST" - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, response, error) -> Void in defer { expect.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + guard let httpResponse = try? XCTUnwrap(response as? HTTPURLResponse) else { + XCTFail("response should be a non-nil HTTPURLResponse") + return + } + XCTAssertNotNil(httpResponse.allHeaderFields["Set-Cookie"]) } task.resume() waitForExpectations(timeout: 30) @@ -593,15 +601,21 @@ class TestURLSession : LoopbackServerTest { func test_cookiesStorage() { let config = URLSessionConfiguration.default config.timeoutIntervalForRequest = 5 + emptyCookieStorage(storage: config.httpCookieStorage) let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) var expect = expectation(description: "POST \(urlString)") var req = URLRequest(url: URL(string: urlString)!) req.httpMethod = "POST" - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, response, error) -> Void in defer { expect.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + guard let httpResponse = try? XCTUnwrap(response as? HTTPURLResponse) else { + XCTFail("response should be a non-nil HTTPURLResponse") + return + } + XCTAssertNotNil(httpResponse.allHeaderFields["Set-Cookie"]) } task.resume() waitForExpectations(timeout: 30) @@ -612,57 +626,81 @@ class TestURLSession : LoopbackServerTest { func test_redirectionWithSetCookies() { let config = URLSessionConfiguration.default config.timeoutIntervalForRequest = 5 - if let storage = config.httpCookieStorage, let cookies = storage.cookies { - for cookie in cookies { - storage.deleteCookie(cookie) - } - } - let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirectSetCookies" + emptyCookieStorage(storage: config.httpCookieStorage) + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirectToEchoHeaders" let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) var expect = expectation(description: "POST \(urlString)") - var req = URLRequest(url: URL(string: urlString)!) - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let req = URLRequest(url: URL(string: urlString)!) + let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } - XCTAssertNotNil(data) + // Because /redirectToEchoHeaders is a redirection, this is the + // final result of the redirection, not the redirection itself. + guard let data = try? XCTUnwrap(data) else { + XCTFail("data should not be nil") + return + } XCTAssertNil(error as? URLError, "error = \(error as! URLError)") - guard let data = data else { return } let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" - print("headers here = \(headers)") XCTAssertNotNil(headers.range(of: "Cookie: redirect=true")) } task.resume() waitForExpectations(timeout: 30) } - func test_setCookies() { + func test_previouslySetCookiesAreSentInLaterRequests() { let config = URLSessionConfiguration.default config.timeoutIntervalForRequest = 5 - let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/setCookies" + emptyCookieStorage(storage: config.httpCookieStorage) let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) - var expect = expectation(description: "POST \(urlString)") - var req = URLRequest(url: URL(string: urlString)!) - req.httpMethod = "POST" - var task = session.dataTask(with: req) { (data, _, error) -> Void in - defer { expect.fulfill() } + + let urlString1 = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" + var expect1 = expectation(description: "POST \(urlString1)") + var req1 = URLRequest(url: URL(string: urlString1)!) + req1.httpMethod = "POST" + + let urlString2 = "http://127.0.0.1:\(TestURLSession.serverPort)/echoHeaders" + var expect2 = expectation(description: "POST \(urlString2)") + var req2 = URLRequest(url: URL(string: urlString2)!) + req2.httpMethod = "POST" + + let task1 = session.dataTask(with: req1) { (data, response, error) -> Void in + defer { expect1.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") - guard let data = data else { return } - let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" - XCTAssertNotNil(headers.range(of: "Cookie: fr=anjd&232")) + guard let httpResponse = try? XCTUnwrap(response as? HTTPURLResponse) else { + XCTFail("response should be a non-nil HTTPURLResponse") + return + } + XCTAssertNotNil(httpResponse.allHeaderFields["Set-Cookie"]) + + let task2 = session.dataTask(with: req2) { (data, _, error) -> Void in + defer { expect2.fulfill() } + guard let data = try? XCTUnwrap(data) else { + XCTFail("data should not be nil") + return + } + XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" + XCTAssertNotNil(headers.range(of: "Cookie: fr=anjd&232")) + } + task2.resume() } - task.resume() + task1.resume() + waitForExpectations(timeout: 30) } - func test_cookieStorageForEphmeralConfiguration() { + func test_cookieStorageForEphemeralConfiguration() { let config = URLSessionConfiguration.ephemeral config.timeoutIntervalForRequest = 5 + emptyCookieStorage(storage: config.httpCookieStorage) + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) var expect = expectation(description: "POST \(urlString)") var req = URLRequest(url: URL(string: urlString)!) req.httpMethod = "POST" - var task = session.dataTask(with: req) { (data, _, error) -> Void in + let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") @@ -677,24 +715,47 @@ class TestURLSession : LoopbackServerTest { XCTAssertEqual(cookies2?.count, 0) } - func test_dontSetCookies() { + func test_setCookieHeadersCanBeIgnored() { let config = URLSessionConfiguration.default config.timeoutIntervalForRequest = 5 config.httpShouldSetCookies = false - let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/setCookies" + emptyCookieStorage(storage: config.httpCookieStorage) let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) - var expect = expectation(description: "POST \(urlString)") - var req = URLRequest(url: URL(string: urlString)!) - req.httpMethod = "POST" - var task = session.dataTask(with: req) { (data, _, error) -> Void in - defer { expect.fulfill() } + + let urlString1 = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" + var expect1 = expectation(description: "POST \(urlString1)") + var req1 = URLRequest(url: URL(string: urlString1)!) + req1.httpMethod = "POST" + + let urlString2 = "http://127.0.0.1:\(TestURLSession.serverPort)/echoHeaders" + var expect2 = expectation(description: "POST \(urlString2)") + var req2 = URLRequest(url: URL(string: urlString2)!) + req2.httpMethod = "POST" + + let task1 = session.dataTask(with: req1) { (data, response, error) -> Void in + defer { expect1.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") - guard let data = data else { return } - let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" - XCTAssertNil(headers.range(of: "Cookie: fr=anjd&232")) + guard let httpResponse = try? XCTUnwrap(response as? HTTPURLResponse) else { + XCTFail("response should be a non-nil HTTPURLResponse") + return + } + XCTAssertNotNil(httpResponse.allHeaderFields["Set-Cookie"]) + + let task2 = session.dataTask(with: req2) { (data, _, error) -> Void in + defer { expect2.fulfill() } + guard let data = try? XCTUnwrap(data) else { + XCTFail("data should not be nil") + return + } + XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" + XCTAssertNil(headers.range(of: "Cookie: fr=anjd&232")) + } + task2.resume() } - task.resume() + task1.resume() + waitForExpectations(timeout: 30) } @@ -749,7 +810,7 @@ class TestURLSession : LoopbackServerTest { var expect = expectation(description: "POST \(urlString): post with empty body") var req = URLRequest(url: URL(string: urlString)!) req.httpMethod = "POST" - var task = session.dataTask(with: req) { (_, response, error) -> Void in + let task = session.dataTask(with: req) { (_, response, error) -> Void in defer { expect.fulfill() } XCTAssertNil(error as? URLError, "error = \(error as! URLError)") guard let httpresponse = response as? HTTPURLResponse else { fatalError() } @@ -763,7 +824,6 @@ class TestURLSession : LoopbackServerTest { let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/unauthorized" let url = URL(string: urlString)! let expect = expectation(description: "GET \(urlString): with a completion handler") - var expectedResult = "unknown" let session = URLSession(configuration: URLSessionConfiguration.default) let task = session.dataTask(with: url) { _, response, error in defer { expect.fulfill() } @@ -1052,9 +1112,9 @@ class TestURLSession : LoopbackServerTest { ("test_concurrentRequests", test_concurrentRequests), ("test_disableCookiesStorage", test_disableCookiesStorage), ("test_cookiesStorage", test_cookiesStorage), - ("test_cookieStorageForEphmeralConfiguration", test_cookieStorageForEphmeralConfiguration), - ("test_setCookies", test_setCookies), - ("test_dontSetCookies", test_dontSetCookies), + ("test_cookieStorageForEphemeralConfiguration", test_cookieStorageForEphemeralConfiguration), + ("test_previouslySetCookiesAreSentInLaterRequests", test_previouslySetCookiesAreSentInLaterRequests), + ("test_setCookieHeadersCanBeIgnored", test_setCookieHeadersCanBeIgnored), ("test_initURLSessionConfiguration", test_initURLSessionConfiguration), ("test_basicAuthRequest", test_basicAuthRequest), ("test_redirectionWithSetCookies", test_redirectionWithSetCookies),