DEV Community

Warren Burton
Warren Burton

Posted on

Always add a test

Im writing about the flow for fixing a bug and committing that fix to the test suite.

My Crashlytics log for Trout OSX was showing this crash:

Alt Text

For some reason when a person went to delete a Link Type entry a crash was happening.

What was going on? A customer selects a link type and presses the delete button to remove it.

Alt Text

Its an arbitrary biz rule to not allow the default type to be deleted.

I had forgotten to bind the enabled property of the delete button to isDefault on the model object. A simple enough fix at UI level in the XIB.

Alt Text

But what about the future? Maybe I redo the app in SwiftUI ? The next task is to fix this in the logic.

The logic to delete looked like this:

@objc func deleteLinkType(_ type: VMLinkType) {

    if let links = type.links {
        let dtype = defaultLinkType()
        for link in links {
            link.type = dtype
        }
    }
    context.delete(type)
    normaliseTypeIndexes()
}

So I inhibit the default type from being deleted, again simple enough.

@objc func deleteLinkType(_ type: VMLinkType) {

    guard type.isDefaultValue == false else { return }

    //...
}

But now how do I capture this decision. I add a test for this condition.

I like using Quick & Nimble as the test language is more natural than base XCTest. So I drag the relevant file in DevSketch and write two tests.

  • delete link type cannot delete default link
  • delete link type can delete non default link

DevSketch generates the boilerplate for me as I can never remember the Quick syntax off the top of my head.

Alt Text

I can drag that boiler plate straight to Xcode and construct the tests.

import Quick
import Nimble
@testable import Trout

class TroutMetadataSpaceTests: QuickSpec {

    override func spec() {

        it("delete link type cannot delete default type") {
            let stack = TestCoreDataStack()
            let metadata = TroutMetadataSpace(context: stack.managedObjectContext)
            guard let defaulttype = metadata.defaultLinkType() else {
                XCTFail()
                return
            }

            metadata.deleteLinkType(defaulttype)
            expect(metadata.defaultLinkType()).toNot(beNil())
        }

        it("delete link type can delete non default type") {
            let stack = TestCoreDataStack()
            let metadata = TroutMetadataSpace(context: stack.managedObjectContext)

            let types = metadata.allLinkTypes
            guard let deletable = types.last else {
                XCTFail()
                return
            }

            metadata.deleteLinkType(deletable)
            let postdeletetypes = metadata.allLinkTypes
            expect(postdeletetypes.contains(deletable)).to(beFalse())
        }
    }
}

Two atomic tests to express the biz logic. Even if I end up refactoring the whole system in the future I have captured the decision.

In summary

  1. Fix the UI bug. Technically I could stop here as the customer won't care.
  2. Fix the underlying logic so that it can't happen again.
  3. Add tests so the decision is captured for the long term good of the project.

Writing the tests might take a few minutes more than just putting a sticking plaster on the UI but now I can be confident it won't get borked in the future.

Top comments (0)