Refactoring with TDD

As everybody knows, good software engineering recommends to respect separation of concerns. For example, controller’s main responsibilities are to process incoming HTTP requests, to invoke some business logic and to dispatch the response.

You can see that the following controller doesn’t respect this recommendation since the searching logic is implemented directly into the controller method.

package todo
 
class TaskController {
 
    static scaffold = true
 
    def list() {
        params.max = Math.min(params.max ? params.int('max') : 5, 100)
 
        def taskList = Task.createCriteria().list (params) {
            if ( params.query ) {
                ilike("description", "%${params.query}%")
            }
        }
 
        [taskInstanceList: taskList, taskInstanceTotal: taskList.totalCount]
    }
 
}

Obviously, it’s a very simple example and most people would say that it’s not a big deal but let me show you how to refactor this code to respect separation of concerns.

The most common places to implement business logic in Grails are services and domain classes. In our case, a service would be unnecessary at this point so let’s move business logic to the Task domain class.

This time, I’ll use Test Driven Development so I’m writing the test first:

// file: test/unit/todo/TaskSpec.groovy
package todo

import grails.test.mixin.TestFor

import spock.lang.Specification
import spock.lang.Unroll

@TestFor(Task)
class TaskSpec extends Specification {

	@Unroll
	def "search() should return #count results for query '#query'"() {
	    given:
            [
                new Task(description: "Buy bread"),
                new Task(description: "Buy milk"),
	        ]*.save()
	    expect:
            count == Task.search(query).count()
	    where:
    	    count   | query
    	    2       | null
    	    2       | "Buy"
    	    1       | "Bread"
    	    0       | "iPod"
	}

}

This is a Spock unit test:

  • it first creates and saves 2 tasks.
  • it then passes a query string to the Task domain class
  • it then asserts the number of matching results for various queries

If you run this test with the command grails test-app unit:spock TaskSpec, you should see the following:

| Running 1 spock test... 1 of 1
| Failure:  search() should return '2' results for query 'null'(todo.TaskSpec)
|  groovy.lang.MissingMethodException: No signature of method: todo.Task.search() is applicable for argument types: (null) values: [null]
[some output omitted]
| Completed 1 spock test, 4 failed in 362ms
| Tests FAILED  - view reports in target/test-reports

All 4 test cases fail as Grails complains that it can’t find any search method in the Task class. Let’s create one with a named query:

package todo

class Task {

    String description

    static namedQueries = {
        search { query ->
        }
    }

}

Here is what Grails outputs if we run the same command again:

| Running 1 spock test... 3 of 1
| Failure:  search() should return '1' results for query 'Bread'(todo.TaskSpec)
|  Condition not satisfied:

count == Task.search(query).count()
|     |       |      |      |
1     false   |      Bread  2
              org.grails.datastore.gorm.query.NamedCriteriaProxy@5d5ef3e7

	at todo.TaskSpec.search() should return '#count' results for query '#query'(TaskSpec.groovy:19)
| Running 1 spock test... 4 of 1
| Failure:  search() should return '0' results for query 'iPod'(todo.TaskSpec)
|  Condition not satisfied:

count == Task.search(query).count()
|     |       |      |      |
0     false   |      iPod   2
              org.grails.datastore.gorm.query.NamedCriteriaProxy@74ef95c6

	at todo.TaskSpec.search() should return '#count' results for query '#query'(TaskSpec.groovy:19)
| Completed 1 spock test, 2 failed in 762ms
| Tests FAILED  - view reports in target/test-reports

OK, no more groovy.lang.MissingMethodException but some test cases still fail. So let’s implement the search named query. All we have to do is to move the business logic from the controller method to the domain class:

package todo

class Task {

    String description

    static namedQueries = {
        search { query ->
            if ( query ) {
                ilike("description", "%${query}%")
            }
        }
    }

}

And now, all test cases pass:

| Completed 1 spock test, 0 failed in 329ms
| Tests PASSED - view reports in target/test-reports

Since we moved the search logic to the domain class, we can now remove it from the controller:

package todo
 
class TaskController {
 
    static scaffold = true
 
    def list() {
        params.max = Math.min(params.max ? params.int('max') : 5, 100)
 
        def taskList = Task.search(params.query).list(params)
 
        [taskInstanceList: taskList, taskInstanceTotal: taskList.totalCount]
    }
 
}

We successfully refactored our controller method to move business logic where it belongs.

Test Grails application with Spock

Because repeated manual testing is long, tedious and error-prone, you should use automated tests to ensure that your application does what it is supposed to do and keeps doing what it did in the past (non-regression).

Grails comes with jUnit bundled in but I’ve enjoyed using Spock instead lately. Spock is great testing framework that lets you write DRY elegant automated tests.

Good news, there is a Grails Spock plugin. Add it to your Grails application with the following line in your BuildConfig file:

plugins {
	...
		test ":spock:0.6"
	...
}

In the previous post, we’ve seen how to add a simple search form to a basic scaffolded controller. Let’s write some automated tests for our searching feature.

Here is our TaskController:

package todo

class TaskController {

    static scaffold = true

    def list() {
        params.max = Math.min(params.max ? params.int('max') : 5, 100)

        def taskList = Task.createCriteria().list (params) {
            if ( params.query ) {
                ilike("description", "%${params.query}%")
            }
        }

        [taskInstanceList: taskList, taskInstanceTotal: taskList.totalCount]
    }

}

Let’s write a unit test for the list method of this controller:

// file: [application]/test/unit/todo/TaskControllerSpec.groovy
package todo

import grails.test.mixin.TestFor
import grails.test.mixin.Mock
import spock.lang.Specification

@TestFor(TaskController)
@Mock(Task)
class TaskControllerSpec extends Specification {

    void "list() should return no results with no records in DB"() {
        given:
            def model = controller.list()
        expect:
            model.taskInstanceList.size() == 0
            model.taskInstanceTotal == 0
    }

}

It’s a simple test, mostly to make sure that everything is wired correctly. Run it with the command grails test-app unit:spock and you should see:

| Completed 1 spock test, 0 failed in 4393ms
| Tests PASSED - view reports in target/test-reports

Since we declared this test as a unit test, we need to mock our Task domain class with the @Mock(Task). This annotation injects the necessary GORM features for our test to run.

We should now test for various combination of data and search queries. One of the nice features of Spock is the possibility to test several hypothesis and results in the same test method with the @Unroll annotation (don’t forget to import it in your test file: import spock.lang.Unroll).

    @Unroll
    void "list() should return '#size' item(s) of '#total' total results with query '#search_query'"() {
        given:
            (1..20).each { i -> new Task(description: "Task #${i}").save() }
            params.query = search_query
        and:
            def model = controller.list()
        expect:
            model.taskInstanceList.size() == size
            model.taskInstanceTotal == total
        where:
            search_query    | size  | total
            null            | 5     | 20
            ""              | 5     | 20
            "no task"       | 0     | 0
            "17"            | 1     | 1
            "Task #1"       | 5     | 11
            "Task"          | 5     | 20
    }

Thanks to the expressive DSL of Spock, this test reads pretty well even if you are not familiar with the framework: given a list of 200 tasks in the database and a search query, the method list of the controller should return the right number of paginated matches out of the total of results.

With the Unroll annotation, this tests will actually acts as 6 separate Spock tests. Each one running with a different set of hypothesis and results, as described in the where: clause.

Finally, let’s test that our method never return more than the 100 results as specified by the first line of the controller method:

    void "list() should never return more than 100 results"() {
        given:
            params.max = 150
            (1..200).each { i -> new Task(description: "Task #${i}").save() }
        and:
            def model = controller.list()
        expect:
            model.taskInstanceList.size() == 100
            model.taskInstanceTotal == 200
    }

Note how Groovy lets us create and save 200 Task instances with a single line of code.

Here if our final Spock test file:

package todo

import grails.test.mixin.TestFor
import grails.test.mixin.Mock

import spock.lang.Specification
import spock.lang.Unroll

@TestFor(TaskController)
@Mock(Task)
class TaskControllerSpec extends Specification {

    void "list() should return no results with an empty database"() {
        given:
            def model = controller.list()
        expect:
            model.taskInstanceList.size() == 0
            model.taskInstanceTotal == 0
    }

    @Unroll
    void "list() should return '#size' item(s) of '#total' total results with query '#search_query'"() {
        given:
            (1..20).each { i -> new Task(description: "Task #${i}").save() }
            params.query = search_query
        and:
            def model = controller.list()
        expect:
            model.taskInstanceList.size() == size
            model.taskInstanceTotal == total
        where:
            search_query    | size  | total
            null            | 5     | 20
            ""              | 5     | 20
            "no task"       | 0     | 0
            "17"            | 1     | 1
            "Task #1"       | 5     | 11
            "Task"          | 5     | 20
    }

    void "list() should never return more than 100 results"() {
        given:
            params.max = 150
            (1..200).each { i -> new Task(description: "Task #${i}").save() }
        and:
            def model = controller.list()
        expect:
            model.taskInstanceList.size() == 100
            model.taskInstanceTotal == 200
    }

}

You can run all the tests with the command grails test-app unit:spock.