buildasaurs / XcodeServerSDK

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

XcodeServerEndpoints.createRequest tests #84

Closed pmkowal closed 9 years ago

pmkowal commented 9 years ago

Hey, I added three tests for createRequest method, but I don't have a clue how to 100% cover the XcodeServerEndpoints class. Please take a look.

Btw. I have one proposition regarding XcodeServer class. I would move the code below (from sendRequestWithMethod method) to createRequest method, because createRequest method can figure out HTTP.Method directly from its parameter. What do you think?

var allParams = [
    "method": method.rawValue
]

//merge the two params
if let params = params {
    for (key, value) in params {
        allParams[key] = value
    }
}
buildasaur commented 9 years ago

Result of Integration 1

Duration: 1 minute and 5 seconds Result: Perfect build! All 48 tests passed. :+1: Test Coverage: 51%.

cojoj commented 9 years ago

@pmkowal can you provide a screenshot of test coverage form XcodeServerEndpoints class?

pmkowal commented 9 years ago

here you go @cojoj:

testcoverage
cojoj commented 9 years ago

Nah, the test coverage in code... You know, this fancy indicators showing which lines has been tested 😉

pmkowal commented 9 years ago

@cojoj are you concerned with buildasaur result of integration?

cojoj commented 9 years ago

Nope, just want to see coverage and give you ans answer to

but I don't have a clue how to 100% cover the XcodeServerEndpoints class

pmkowal commented 9 years ago

Cool, I didn't look at it, thx @cojoj

linescoverage
pmkowal commented 9 years ago

Ok, I know now how to test authorization part, but do you have idea how to test body part?

cojoj commented 9 years ago

Great I was just writing about his authorization part but if you know how to solve this good for you! 🍹

Now, the body part... It'll be used [mostly] for POST actions. In that case you have to provide a body parameter to createRequest(). To fully check this you'll have to provide a valid JSON and in that case try wil pass and everything will succeed. To cover rest you'll have to provide a failbale JSON which will crash NSJSONSerialization.dataWithJSONObject()

Hope you get me 😉

pmkowal commented 9 years ago

The point is tried to provide failbale JSON, but I couldn't figure it out :smile:, because we can use only [String: String] as body param.

pmkowal commented 9 years ago

@cojoj ...and with authorization I would create another instance of XcodeServerEndpoints with different config. Do you have other ideas?

cojoj commented 9 years ago

Exactly... That's what I've been struggling with too 😕 One way is to provide a malformed String but right now I can't think of any... Leaving code like this without a coverage is a bummer but I guess right now we have no other choice. Maybe @czechboy0 have a neat solution to this? 😜


You can move XcodeServerEndpoint to a property and mark it as var so later in test cases you can simply replace config.

cojoj commented 9 years ago

I'd be great if we have read access to Foundation... You'd simply check what Apple understands under failable JSON. Right now IDK... ¯(ツ)

pmkowal commented 9 years ago

You can move XcodeServerEndpoint to a property and mark it as var so later in test cases you can simply replace config

I was rather thinking of creating another XcodeServerEndpoints in this particular method, because replacing serverConfig could break other tests imo. The seond thing is we cannot exchange serverConfig since it is declared as a let.

I'd be great if we have read access to Foundation... You'd simply check what Apple understands under failable JSON. Right now IDK... ¯(ツ)

:grin:

cojoj commented 9 years ago

Oh okay... If it's let you have to create instance of XcodeServerEnspoints for each test case.

buildasaur commented 9 years ago

Result of Integration 2

Duration: 52 seconds Result: Perfect build! All 49 tests passed. :+1: Test Coverage: 52%.

pmkowal commented 9 years ago

ok @cojoj authorization part updated.

czechboy0 commented 9 years ago

Great stuff, thanks @pmkowal!

pmkowal commented 9 years ago

:+1: