Repository: Alamofire/Alamofire
File: Tests/DownloadTests.swift (Swift)

Code before:
// DownloadTests.swift
//
// Copyright (c) 2014–2015 Alamofire Software Foundation (http://alamofire.org/)
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import Alamofire
import Foundation
import XCTest

class DownloadResponseTestCase: BaseTestCase {
    // MARK: - Properties

    let searchPathDirectory: NSSearchPathDirectory = .CachesDirectory
    let searchPathDomain: NSSearchPathDomainMask = .UserDomainMask

    // MARK: - Tests

    func testDownloadRequest() {
        // Given
        let numberOfLines = 100
        let URLString = "http://httpbin.org/stream/\(numberOfLines)"

        let destination = Alamofire.Request.suggestedDownloadDestination(directory: searchPathDirectory, domain: searchPathDomain)

        let expectation = expectationWithDescription("Download request should download data to file: \(URLString)")

        var request: NSURLRequest?
        var response: NSHTTPURLResponse?
        var error: NSError?

        // When
        Alamofire.download(.GET, URLString, destination)
            .response { responseRequest, responseResponse, _, responseError in
                request = responseRequest
                response = responseResponse
                error = responseError

                expectation.fulfill()
        }

        waitForExpectationsWithTimeout(self.defaultTimeout, handler: nil)

        // Then
        XCTAssertNotNil(request, "request should not be nil")
        XCTAssertNotNil(response, "response should not be nil")
        XCTAssertNil(error, "error should be nil")

        let fileManager = NSFileManager.defaultManager()
        let directory = fileManager.URLsForDirectory(self.searchPathDirectory, inDomains: self.searchPathDomain)[0] as! NSURL

        var fileManagerError: NSError?
        if let contents = fileManager.contentsOfDirectoryAtURL(directory, includingPropertiesForKeys: nil, options: .SkipsHiddenFiles, error: &fileManagerError) {
            XCTAssertNil(fileManagerError, "fileManagerError should be nil")

            #if os(iOS)
            let suggestedFilename = "\(numberOfLines)"
            #elseif os(OSX)
            let suggestedFilename = "\(numberOfLines).json"
            #endif

            let predicate = NSPredicate(format: "lastPathComponent = '\(suggestedFilename)'")
            let filteredContents = (contents as NSArray).filteredArrayUsingPredicate(predicate)
            XCTAssertEqual(filteredContents.count, 1, "should have one file in Documents")

            if let file = filteredContents.first as? NSURL {
                XCTAssertEqual(file.lastPathComponent ?? "", "\(suggestedFilename)", "filename should be \(suggestedFilename)")

                if let data = NSData(contentsOfURL: file) {
                    XCTAssertGreaterThan(data.length, 0, "data length should be non-zero")
                } else {
                    XCTFail("data should exist for contents of URL")
                }

                fileManager.removeItemAtURL(file, error: nil)
            } else {
                XCTFail("file should not be nil")
            }
        } else {
            XCTFail("contents should not be nil")
        }
    }

    func testDownloadRequestWithProgress2() {
        // Given
        let randomBytes = 4 * 1024 * 1024
        let URLString = "http://httpbin.org/bytes/\(randomBytes)"

        let fileManager = NSFileManager.defaultManager()
        let directory = fileManager.URLsForDirectory(self.searchPathDirectory, inDomains: self.searchPathDomain)[0] as! NSURL
        let filename = "test_download_data"
        let fileURL = directory.URLByAppendingPathComponent(filename)

        let expectation = expectationWithDescription("Bytes download progress should be reported: \(URLString)")

        var byteValues: [(bytes: Int64, totalBytes: Int64, totalBytesExpected: Int64)] = []
        var progressValues: [(completedUnitCount: Int64, totalUnitCount: Int64)] = []
        var responseRequest: NSURLRequest?
        var responseResponse: NSHTTPURLResponse?
        var responseData: AnyObject?
        var responseError: NSError?

        // When
        let download = Alamofire.download(.GET, URLString) { _, _ in
            return fileURL
        }
        download.progress { bytesRead, totalBytesRead, totalBytesExpectedToRead in
            let bytes = (bytes: bytesRead, totalBytes: totalBytesRead, totalBytesExpected: totalBytesExpectedToRead)
            byteValues.append(bytes)

            let progress = (completedUnitCount: download.progress.completedUnitCount, totalUnitCount: download.progress.totalUnitCount)
            progressValues.append(progress)
        }
        download.response { request, response, data, error in
            responseRequest = request
            responseResponse = response
            responseData = data
            responseError = error

            expectation.fulfill()
        }

        waitForExpectationsWithTimeout(self.defaultTimeout, handler: nil)

        // Then
        XCTAssertNotNil(responseRequest, "response request should not be nil")
        XCTAssertNotNil(responseResponse, "response response should not be nil")
        XCTAssertNil(responseData, "response data should be nil")
        XCTAssertNil(responseError, "response error should be nil")

        XCTAssertEqual(byteValues.count, progressValues.count, "byteValues count should equal progressValues count")

        if byteValues.count == progressValues.count {
            for index in 0..<byteValues.count {
                let byteValue = byteValues[index]
                let progressValue = progressValues[index]

                XCTAssertGreaterThan(byteValue.bytes, 0, "reported bytes should always be greater than 0")
                XCTAssertEqual(byteValue.totalBytes, progressValue.completedUnitCount, "total bytes should be equal to completed unit count")
                XCTAssertEqual(byteValue.totalBytesExpected, progressValue.totalUnitCount, "total bytes expected should be equal to total unit count")
            }
        }

        if let lastByteValue = byteValues.last,
            lastProgressValue = progressValues.last
        {
            let byteValueFractionalCompletion = Double(lastByteValue.totalBytes) / Double(lastByteValue.totalBytesExpected)
            let progressValueFractionalCompletion = Double(lastProgressValue.0) / Double(lastProgressValue.1)

            XCTAssertEqual(byteValueFractionalCompletion, 1.0, "byte value fractional completion should equal 1.0")
            XCTAssertEqual(progressValueFractionalCompletion, 1.0, "progress value fractional completion should equal 1.0")
        } else {
            XCTFail("last item in bytesValues and progressValues should not be nil")
        }

        var removalError: NSError?
        fileManager.removeItemAtURL(fileURL, error: &removalError)
        XCTAssertNil(removalError, "removal error should be nil")
    }
}


Review comment (bug):
#### Name Change

I'm on it...good catch!

#### Repeating Yourself in Tests

I agree. I don't think there's any issue with repeating yourself in tests. I think it's advantageous to use a small amount of inheritance for simple things, but otherwise just repeat the logic so the majority of the code resides directly in the test or at least in the same test file. The more test logic that resides directly in the test method, the easier it is to quickly see all the moving parts of the test.

#### Test Individual Things

Great question @kcharwood. I'm going to try to break down my testing approach here and would love everyone's feedback.

I feel that the philosophy of "Test Individual Things" is a bit outdated when applied to one test, one assert. XCTest can continue running your test even if you trip an assertion. It's perfectly fine to have multiple assertions within the same test, it will report them all to you. As long as you have a policy of always keeping your test suite green, why does it matter that you have one test that actually has multiple failures? If your test suite cannot continue if it fails on an assertion, then I completely agree with the philosophy. 

With that said, I do believe there's merit in attempting to write more focused tests. IMO, tests should focus on a particular operation and break it down into as small of unit as possible, then test the results. Next, you create a new set of input parameters, run the operation again, test the results. Rinse and repeat. Testing individual things to me means "test individual operations", not being limited to only have a single assertion in your test.

I also think this philosophy tends to break down with integration tests. Let's take the download tests for example. I just counted 23 different asserts in those two methods, some of which are called upwards of 20 times in a for loop. If we broke this out into individual tests, we'd be looking at thousands of lines of code to test the same logic. When you end up having to repeat yourself that many times, you make copy/paste errors and I think it makes your tests more fragile. I also believe it makes it difficult to understand what is really being tested since almost every test looks the same. Good descriptive names certainly help, but it gets super repetitive to anyone that didn't actually write the tests in the first place.

I could certainly be convinced in those download tests that we should have individual tests more like the following:
1. Successful download request (test completion handler arguments)
2. Failed download request due to bad URL (test completion handler arguments)
3. Failed download request due to bad file path (test completion handler arguments)
4. Successful download request testing only the progress values
5. Failed download request testing only the progress values

It's difficult for me to see how splitting each of these tests up into 4 or 5 additional methods to test each of these parameters would be a "better" overall approach to appease the testing gods. Especially in the case of the progress tests. Additionally, we wouldn't just be able to move the common logic into a "setup" type method, because the majority of the setup parameters get passed into the actual function being tested. While we could pass back a tuple back with upwards of 5 or 6 parameters inside it, I think our tuples would be getting a bit out of control.

One final point that seems a bit strange to me is how we can actually confirm that our single assertion test really did pass successfully if we don't test all the results that we expected? For example, if I make a data request, I expect that the payload data will exist, and I won't receive an error. If I split these into two different tests, how do I know that it is truly working? What if I'm for some reason I am actually receiving both data and an error, and the way I wrote my tests makes it unlikely that I would be able to catch that condition? That may not be the best example, but hopefully the idea of that issue came across. Dynamic operations, such as networking, could benefit from testing all resulting values in the operation rather than a lone expected one to always cover the "unexpected" behaviors.

In summary, I'm all for writing the best tests possible. I'd love everyone's feedback and would love to find a testing strategy that everyone agrees on and is happy with. My two cents...


Code after:
// DownloadTests.swift
//
// Copyright (c) 2014–2015 Alamofire Software Foundation (http://alamofire.org/)
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import Alamofire
import Foundation
import XCTest

class DownloadResponseTestCase: BaseTestCase {
    // MARK: - Properties

    let searchPathDirectory: NSSearchPathDirectory = .CachesDirectory
    let searchPathDomain: NSSearchPathDomainMask = .UserDomainMask

    // MARK: - Tests

    func testDownloadRequest() {
        // Given
        let numberOfLines = 100
        let URLString = "http://httpbin.org/stream/\(numberOfLines)"

        let destination = Alamofire.Request.suggestedDownloadDestination(directory: searchPathDirectory, domain: searchPathDomain)

        let expectation = expectationWithDescription("Download request should download data to file: \(URLString)")

        var request: NSURLRequest?
        var response: NSHTTPURLResponse?
        var error: NSError?

        // When
        Alamofire.download(.GET, URLString, destination)
            .response { responseRequest, responseResponse, _, responseError in
                request = responseRequest
                response = responseResponse
                error = responseError

                expectation.fulfill()
        }

        waitForExpectationsWithTimeout(self.defaultTimeout, handler: nil)

        // Then
        XCTAssertNotNil(request, "request should not be nil")
        XCTAssertNotNil(response, "response should not be nil")
        XCTAssertNil(error, "error should be nil")

        let fileManager = NSFileManager.defaultManager()
        let directory = fileManager.URLsForDirectory(self.searchPathDirectory, inDomains: self.searchPathDomain)[0] as! NSURL

        var fileManagerError: NSError?
        if let contents = fileManager.contentsOfDirectoryAtURL(directory, includingPropertiesForKeys: nil, options: .SkipsHiddenFiles, error: &fileManagerError) {
            XCTAssertNil(fileManagerError, "fileManagerError should be nil")

            #if os(iOS)
            let suggestedFilename = "\(numberOfLines)"
            #elseif os(OSX)
            let suggestedFilename = "\(numberOfLines).json"
            #endif

            let predicate = NSPredicate(format: "lastPathComponent = '\(suggestedFilename)'")
            let filteredContents = (contents as NSArray).filteredArrayUsingPredicate(predicate)
            XCTAssertEqual(filteredContents.count, 1, "should have one file in Documents")

            if let file = filteredContents.first as? NSURL {
                XCTAssertEqual(file.lastPathComponent ?? "", "\(suggestedFilename)", "filename should be \(suggestedFilename)")

                if let data = NSData(contentsOfURL: file) {
                    XCTAssertGreaterThan(data.length, 0, "data length should be non-zero")
                } else {
                    XCTFail("data should exist for contents of URL")
                }

                fileManager.removeItemAtURL(file, error: nil)
            } else {
                XCTFail("file should not be nil")
            }
        } else {
            XCTFail("contents should not be nil")
        }
    }

    func testDownloadRequestWithProgress() {
        // Given
        let randomBytes = 4 * 1024 * 1024
        let URLString = "http://httpbin.org/bytes/\(randomBytes)"

        let fileManager = NSFileManager.defaultManager()
        let directory = fileManager.URLsForDirectory(self.searchPathDirectory, inDomains: self.searchPathDomain)[0] as! NSURL
        let filename = "test_download_data"
        let fileURL = directory.URLByAppendingPathComponent(filename)

        let expectation = expectationWithDescription("Bytes download progress should be reported: \(URLString)")

        var byteValues: [(bytes: Int64, totalBytes: Int64, totalBytesExpected: Int64)] = []
        var progressValues: [(completedUnitCount: Int64, totalUnitCount: Int64)] = []
        var responseRequest: NSURLRequest?
        var responseResponse: NSHTTPURLResponse?
        var responseData: AnyObject?
        var responseError: NSError?

        // When
        let download = Alamofire.download(.GET, URLString) { _, _ in
            return fileURL
        }
        download.progress { bytesRead, totalBytesRead, totalBytesExpectedToRead in
            let bytes = (bytes: bytesRead, totalBytes: totalBytesRead, totalBytesExpected: totalBytesExpectedToRead)
            byteValues.append(bytes)

            let progress = (completedUnitCount: download.progress.completedUnitCount, totalUnitCount: download.progress.totalUnitCount)
            progressValues.append(progress)
        }
        download.response { request, response, data, error in
            responseRequest = request
            responseResponse = response
            responseData = data
            responseError = error

            expectation.fulfill()
        }

        waitForExpectationsWithTimeout(self.defaultTimeout, handler: nil)

        // Then
        XCTAssertNotNil(responseRequest, "response request should not be nil")
        XCTAssertNotNil(responseResponse, "response response should not be nil")
        XCTAssertNil(responseData, "response data should be nil")
        XCTAssertNil(responseError, "response error should be nil")

        XCTAssertEqual(byteValues.count, progressValues.count, "byteValues count should equal progressValues count")

        if byteValues.count == progressValues.count {
            for index in 0..<byteValues.count {
                let byteValue = byteValues[index]
                let progressValue = progressValues[index]

                XCTAssertGreaterThan(byteValue.bytes, 0, "reported bytes should always be greater than 0")
                XCTAssertEqual(byteValue.totalBytes, progressValue.completedUnitCount, "total bytes should be equal to completed unit count")
                XCTAssertEqual(byteValue.totalBytesExpected, progressValue.totalUnitCount, "total bytes expected should be equal to total unit count")
            }
        }

        if let lastByteValue = byteValues.last,
            lastProgressValue = progressValues.last
        {
            let byteValueFractionalCompletion = Double(lastByteValue.totalBytes) / Double(lastByteValue.totalBytesExpected)
            let progressValueFractionalCompletion = Double(lastProgressValue.0) / Double(lastProgressValue.1)

            XCTAssertEqual(byteValueFractionalCompletion, 1.0, "byte value fractional completion should equal 1.0")
            XCTAssertEqual(progressValueFractionalCompletion, 1.0, "progress value fractional completion should equal 1.0")
        } else {
            XCTFail("last item in bytesValues and progressValues should not be nil")
        }

        var removalError: NSError?
        fileManager.removeItemAtURL(fileURL, error: &removalError)
        XCTAssertNil(removalError, "removal error should be nil")
    }
}