Increase test coverage #45

czechboy0 · 2015-06-27T16:56:05Z

Right now our code coverage is somewhere around 15%. ⚡

cojoj · 2015-06-27T17:08:40Z

Well, that's not good but I there are some methods in which I see no sense of covering them with unit tests. What's more, some methods are't totally clear for me because of their dependencie, so I skip them as I cn't write propert tests for them 😭

I was thinking this morning about integrating Travis-CI. What'd you say @czechboy0?

Also, found this cool thing called coveralls 😃

czechboy0 · 2015-06-27T20:12:39Z

They still don't have Xcode 7 support.

cojoj · 2015-06-27T20:15:30Z

czechboy0 · 2015-06-27T20:16:21Z

Their sluggish new Xcode version support is the whole reason Buildasaur exists 😆

czechboy0 · 2015-06-27T20:16:54Z

But yeah once Travis has Xcode 7 support, I'd be up for using it for testing and coverage. No idea how long that will take however.

czechboy0 · 2015-06-27T20:17:36Z

In the meantime we have Xcode 7's code coverage metric, so run it locally and report it in each PR, so that we know what we're at.

cojoj · 2015-06-27T20:19:18Z

There's no school like the oldschool... 😆

cojoj · 2015-06-29T06:15:49Z

I've seen test coverage for XcodeServer but all of them have DEV_ prefix so they're not rally doing anything at the moment. @czechboy0 is this on purpose? I wonder if DVR integration is ready and I can use it to implement new test cases?

czechboy0 · 2015-06-29T07:24:54Z

Yes, the DEV_ prefix is there on purpose. Those are live tests that actually affect and call the real server and I use it during development. So they need to be disabled most of the time unless you're testing live API calls.

And yes, DVR is ready. Just run carthage update --no-build and you're all set. I also added a folder just for Cassettes in the test target/folder.

cojoj · 2015-06-29T07:38:30Z

Hmm, any possibility you write steps needed to use DVR? I how now idea where to put my hands on 😞

czechboy0 · 2015-06-29T08:03:52Z

Look at test DEV_testLive_FetchAndRecordBot() in XcodeServerTests.

Create a test like that for whatever you want to test. Then you run it once and it records and crashes, printing out where it has saved the cassette. You copy that file into the test target in the Cassettes folder, add it to the project to both test targets only and then run it again. This time it will run fine, using the loaded cassette.

cojoj · 2015-06-29T10:40:18Z

Great, but it's easy for GETs, so how about POSTs? Do I have to manually remove added entities?

cojoj · 2015-06-30T06:36:27Z

With #48 we get 25% of test coverage for XcodeServerSDK... Slowly getting more and more 👍

esttorhe · 2015-06-30T23:52:25Z

Have we checked asking for CircleCI access instead of Travis maybe?

cojoj · 2015-07-01T05:11:57Z

Isn't CirlceCI paid?

czechboy0 · 2015-07-01T07:16:08Z

Yeah I tried them all yesterday. They don't yet support Xcode 7 :( Classic.

cojoj · 2015-07-01T07:18:07Z

Travis today has confirmed official support from 6.4...

czechboy0 · 2015-07-01T07:20:57Z

Link? But that doesn't help anyway. We need Xcode 7. Hopefully I'll be able to release an experimental build of Buildasaur soon locally on my Mac to get PR testing again.

cojoj · 2015-07-01T07:22:08Z

We just added support for Xcode 6.4 and the iOS 8.4 SDK! Add `osx_image: xcode6.4` to your .travis.yml to begin using it.
— Travis CI (@TravisCI) czerwiec 30, 2015

esttorhe · 2015-07-01T18:08:57Z

CircleCI is free if we use it only for 1 job at a time (which i think would be fine) except there's no Xcode 7 love yet.

how about Bitrise ? (just checked, they support up to 6.4)

cojoj · 2015-07-01T18:20:21Z

I guess we'll have to wait for anything to support Xcode 7... Other possibility - someone has to allow us to access OS X Server 5.

esttorhe · 2015-07-01T18:23:24Z

We can use Ship.io; they have Xcode 7 support already https://ship.io/xcode-7-beta-is-here/

They have a free plan for individual accounts with 2 jobs or something along those lines; might be worth looking at it

cojoj · 2015-07-01T18:24:24Z

👌

czechboy0 · 2015-07-01T18:36:12Z

Just FYI, I do have an Xcode Bot running locally which tests the swift-2 branch after every single commit, so no worries there. I turned on email notifications so you'll also get them if something breaks.

Unfortunately, we need Pull Request testing, which Buildasaur has (I'm working on Xcode 7 support there) and CI services like Travis do as well, but they don't yet support Xcode 7. It's exactly what happened last year. Ship.io doesn't give us any benefit over a local Xcode Bot. Plus, their Scheme detection is broken, I tried it yesterday.

So once Buildasaur has an Xcode 7 version ready I'll turn it on for this repo so that all PRs will get tested.

esttorhe · 2015-07-01T19:09:00Z

👏 that sounds great then 🎉

TBH i didn't like Ship.io when I tried it.

If I can help with bringing Xcode 7 support to Buildasaur just let me know

esttorhe · 2015-07-01T21:32:53Z

Am I the only one seeing weird behavior with code coverage?

Yesterday I added a test that specifically triggers the invalid scheme path of code but this is what i'm seeing after running that tests:

Is marked as never called

cojoj · 2015-07-01T21:40:59Z

That's really strange... Was it covering the failable path yesterday? Looks fine to me...

Nothing has changed in those files AFAIK.

esttorhe · 2015-07-01T21:42:29Z

It wasn't covered and that's why I went and wrote a test for it but didn't check. now I came to check and is still uncovered.

No matter if I run only that test or all the tests on iOS or OSX that path keeps being marked as not checked.

Perhaps the throws thing has something to do with it?

esttorhe · 2015-07-01T21:43:08Z

Disregard my last comment… this invalidates my theory

guard let url = NSURL(string: host) else {
            /*******************************************************************
             **   Had to be added to silence the compiler ¯\_(ツ)_/¯
             **   Radar: http://openradar.me/21514477
             **   Reply: https://twitter.com/jckarter/status/613491369311535104
             ******************************************************************/
            self.host = ""; self.user = nil; self.password = nil

            throw ConfigurationErrors.InvalidHostProvided(host)
        }

This is passing and marked as checked

czechboy0 · 2015-07-01T22:37:40Z

Just FYI, current state of things:

esttorhe · 2015-07-01T22:40:36Z

That's weird; if you check at the tests and the coverage XcodeServerConfig should be near 100% but the coverage is not working as expected

czechboy0 · 2015-07-01T22:42:54Z

It might also have bugs. We'll see throughout the summer. We know we're still not testing most of the parsing/dictionarifying logic anyway :) I guess once we're confident about that we can tackle these weirdnesses.

cojoj · 2015-07-02T09:33:32Z

Hmmm this whole test coverage works fine for me everytime but bugs are possible...

cojoj · 2015-07-02T10:01:50Z

@czechboy0, @esttorhe I wonder...
Who's the first one to strat writing tests for API calls? 😜

czechboy0 · 2015-07-02T10:04:05Z

All the tools are there. Here is an example of a working test and how easily you get a recording Server. And here is how it works.

czechboy0 · 2015-07-02T10:05:21Z

And here is how you can reuse a recorded Cassette to add many parsing tests.

cojoj · 2015-07-02T10:05:25Z

Did you just point at me? 😜

czechboy0 · 2015-07-02T10:06:47Z

👈 👉 👈 👉 👇 ☝️ 👇 ☝️

Not pointing at anyone, just 💃

cojoj · 2015-07-02T10:09:57Z

cojoj · 2015-07-02T10:18:04Z

On thing I've noticed, you use:

let config = try! XcodeServerConfig(
            host: "https://127.0.0.1",
            user: "ICanCreateBots",
            password: "superSecr3t")

For sake of consistency, should we use default XcodeServerConfig or pass ours? I think the first option will do better as all tests will have the same XcodeServerConfig but in this case all of us will have to create ICanCreateBots user with superSecr3t password.

czechboy0 · 2015-07-02T10:31:53Z

Depends how your XCS is setup, but I have that anyone can view bots and only logged in users can create bots. Which means that any user, including the non-existent ICanCreateBots will return all read-only requests. So you don't need to create this user at all to test most get requests.

When it comes to post requests, I haven't decided yet how we're going to test that since in post requests it's what we're sending is what we want to test, whereas with get requests it's what you're receiving that you want to test.

TL;DR - enable any user to read bots on your XCS and let's write tests for all the get requests first. I'll think of a good way to test posts later. Aight?

cojoj · 2015-07-02T12:01:19Z

I can't agree with you... While testing POST requests it's necessary to assert request but response is also important!

cojoj · 2015-07-02T12:54:57Z

@czechboy0 strange thing happened... I've written test for getRepository() which looks like:

func testGetRepositories() {
        let expectation = self.expectationWithDescription("Get Repositories")
        let server = self.getRecordingXcodeServer("get_repositories")

        server.getRepositories() { (repositories, error) in
            XCTAssertNil(error, "Error should be nil")
            XCTAssertNotNil(repositories, "Repositories shouldn't be nil")

            if let repos = repositories {
                XCTAssertEqual(repos.count, 2, "There should be two repositories available")

                for (index, repo) in repos.enumerate() {
                    XCTAssertEqual(repo.name, "Test\(index + 1)")
                }
            }

            expectation.fulfill()
        }

        self.waitForExpectationsWithTimeout(10) { error in
            if let error = error {
                print("Timeout error: \(error.localizedDescription)")
            }
        }
    }

So at first I had to move get_repositories.json and re-run tests - fine, works and even test is passing but when I look at code coverage I'm like WTF, bro!?:

I can agree that the fragment where completion with nil is passed may not be executed but [Repository] parsing?

I set a breakpoint where the code isn't covered and it executes so this is being called but marked differently 😓

cojoj · 2015-07-02T20:27:31Z

@esttorhe seems like I've been having similiar kind of issue with guard statements...

esttorhe · 2015-07-02T20:28:30Z

Yeah; I saw your tweet to Joe; seems like radar time!! 👯

cojoj · 2015-07-02T20:34:55Z

cojoj · 2015-07-02T20:39:54Z

New follower...

Things just got serious! 😁

czechboy0 · 2015-07-02T21:08:34Z

🎆

czechboy0 · 2015-07-24T19:48:47Z

Getting around 50% now. Not great but getting much better, especially thanks to @cojoj!

czechboy0 · 2015-09-27T21:37:02Z

This is nice, we had fun with it, but it's not really actionable. Closing.

czechboy0 added help wanted task up-for-grabs labels Jun 27, 2015

czechboy0 added this to the Well Tested milestone Jun 27, 2015

esttorhe mentioned this issue Jul 1, 2015

Added missing Repository.swift to iOS target. #56

Merged

czechboy0 added the tests label Jul 1, 2015

cojoj mentioned this issue Jul 2, 2015

Add test for fetching repositories #63

Merged

czechboy0 closed this as completed Sep 27, 2015

Increase test coverage #45

Increase test coverage #45

Comments

czechboy0 commented Jun 27, 2015

cojoj commented Jun 27, 2015

czechboy0 commented Jun 27, 2015

cojoj commented Jun 27, 2015

czechboy0 commented Jun 27, 2015

czechboy0 commented Jun 27, 2015

czechboy0 commented Jun 27, 2015

cojoj commented Jun 27, 2015

cojoj commented Jun 29, 2015

czechboy0 commented Jun 29, 2015

cojoj commented Jun 29, 2015

czechboy0 commented Jun 29, 2015

cojoj commented Jun 29, 2015

cojoj commented Jun 30, 2015

esttorhe commented Jun 30, 2015

cojoj commented Jul 1, 2015

czechboy0 commented Jul 1, 2015

cojoj commented Jul 1, 2015

czechboy0 commented Jul 1, 2015

cojoj commented Jul 1, 2015

esttorhe commented Jul 1, 2015

cojoj commented Jul 1, 2015

esttorhe commented Jul 1, 2015

cojoj commented Jul 1, 2015

czechboy0 commented Jul 1, 2015

esttorhe commented Jul 1, 2015

esttorhe commented Jul 1, 2015

cojoj commented Jul 1, 2015

esttorhe commented Jul 1, 2015

esttorhe commented Jul 1, 2015

czechboy0 commented Jul 1, 2015

esttorhe commented Jul 1, 2015

czechboy0 commented Jul 1, 2015

cojoj commented Jul 2, 2015

cojoj commented Jul 2, 2015

czechboy0 commented Jul 2, 2015

czechboy0 commented Jul 2, 2015

cojoj commented Jul 2, 2015

czechboy0 commented Jul 2, 2015

cojoj commented Jul 2, 2015

cojoj commented Jul 2, 2015

czechboy0 commented Jul 2, 2015

cojoj commented Jul 2, 2015

cojoj commented Jul 2, 2015

cojoj commented Jul 2, 2015

esttorhe commented Jul 2, 2015

cojoj commented Jul 2, 2015

cojoj commented Jul 2, 2015

czechboy0 commented Jul 2, 2015

czechboy0 commented Jul 24, 2015

czechboy0 commented Sep 27, 2015