buildasaurs / XcodeServerSDK

Access Xcode Server API with native Swift objects.
MIT License
399 stars 30 forks source link

Device and Platforms tests #65

Closed cojoj closed 9 years ago

cojoj commented 9 years ago

@czechboy0 I hope you're back from holidays, full of energy and ready to work 😆

New batch with Device tests.

First of all, I've added missing fields/properties to Device model - please check them and let me know what do you think and if things marked as Enum? in comments should be moved to DeviceSpecyfication file.

One thing that concerns me about cassetes is that we're only testing gold path and skipping any possible error paths. Do you think we should create test cases and test failing paths as well? If so, how would you write those tests? Record another casette, write mocks or stubs? As we leave it as it is now we won't get full coverage of the code...

cojoj commented 9 years ago

Platforms tests

Same thing here with testing different execution paths...

buildasaur commented 9 years ago

Result of integration 1 Integration took 47 seconds. Perfect build! All 25 tests passed. :+1:

czechboy0 commented 9 years ago

Let me know when you push the changes so that we can merge this :wink:

cojoj commented 9 years ago

Ok, changed something, added other things and here's the result... 😉

You haven't answered how to handle failing paths in test cases. Got any idea?

buildasaur commented 9 years ago

Result of integration 2 Integration took 34 seconds. Perfect build! All 25 tests passed. :+1:

czechboy0 commented 9 years ago

Yeah the failing paths are interesting. There is not that much code different for each endpoint. Most of the logic is shared. So I'd rather write tests for XcodeServer's sendRequestWithMethod instead of writing very similar failing paths for each API call. Whadaya think?

czechboy0 commented 9 years ago

Cool, thanks as always, @cojoj!

cojoj commented 9 years ago

Exactly, one complete test case for sendRequestWithMethod() can solve all of our problems with specyfic test cases but to write this test we need to create stub so we can imitate the failing response. You know what I mean?

czechboy0 commented 9 years ago

Yup, feel free to use any of the available ones, but it needs to be clear we're testing sendRequestWithMethod, not e.g. getBots, if you know what I mean.

cojoj commented 9 years ago

I guess I'd be hard to achieve as sendRequestWithMethod() has internal accessory flag...

czechboy0 commented 9 years ago

Should be fine with @testable import XcodeServerSDK in the test file, shouldn't it?

cojoj commented 9 years ago

Probably... I'll try to write something

czechboy0 commented 9 years ago

C'mon, http://natashatherobot.com/swift-2-xcode-7-unit-testing-access/

cojoj commented 9 years ago

Hah, I think that testing sendRequestWithMethod() won't solve everything. I think what needs to be tested is createRequest() and sendRequest() (which leads to dataTaskWithRequest()).

czechboy0 commented 9 years ago

Why? Just call it on an XcodeServer you get from getRecordingXcodeServer, just like with the other tests.

The only thing sendRequestWithMethod does is basically call createRequest and right away sendRequest on it :blush: Or am I missing something?

cojoj commented 9 years ago

Right, it doesn't have wide range of responsibilities but you have to fake HTTP responses to check the failable path eg. To test this code (from sendRequestWithMethod()):

guard let data = data else {
    //no body, but a valid response
    completion(response: httpResponse, body: nil, error: nil)
    return
}

You have to return nil body from self.session.dataTaskWithRequest() and to do this you need to provide prepared NSURLRequest which will return proper response...

Correct me I you think that I'm spinning a yarn...

czechboy0 commented 9 years ago

Ok this makes sense. I guess we should test with separate createRequest and sendRequest in this case and with sendRequestWithMethod for different cases, such as with different error returned, server unavailable, wrong returned content types etc.

cojoj commented 9 years ago

Cool!

Anyway, I hate the idea that all in all we'll have to create manually some fake data 😓

czechboy0 commented 9 years ago

:crying_cat_face: