Proper (and Simplified) Testing of a Data Source

Issue

I've recently started getting into testing (TDD) and was wondering if anyone can shed some light into the practice I'm doing. For example, I'm checking if the location provider is available, I implement a contract (data source) class and a wrapper, like so:

LocationDataSource.kt

interface LocationDataSource {

  fun isAvailable(): Observable<Boolean>

}

LocationUtil.kt

class LocationUtil(manager: LocationManager): LocationDataSource {

  private var isAvailableSubject: BehaviorSubject<Boolean> = 
      BehaviorSubject.createDefault(manager.isProviderEnabled(provider))

  override fun isAvailable(): Observable<Boolean> = locationSubject

}

Now, when testing, I'm not sure how to proceed. First thing that I did was mocking the LocationManager and the isProviderEnabled method:

class LocationTest {

  @Mock
  private lateinit var context: Context

  private lateinit var dataSource: LocationDataSource
  private lateinit var manager: LocationManager

  private val observer = TestObserver<Boolean>()

  @Before
  @Throws(Exception::class)
  fun setUp(){
    MockitoAnnotations.initMocks(this)

    // override schedulers here

    `when`(context.getSystemService(LocationManager::class.java))
        .thenReturn(mock(LocationManager::class.java))

    manager = context.getSystemService(LocationManager::class.java)
    dataSource = LocationUtil(manager)
  }

  @Test
  fun isProviderDisabled_ShouldReturnFalse(){
    // Given
    `when`(manager.isProviderEnabled(anyString())).thenReturn(false)

    // When
    dataSource.isLocationAvailable().subscribe(observer)

    // Then
    observer.assertNoErrors()
    observer.assertValue(false)
  }

}

This works. However, during my research on how to do this and that, the time I spent figuring out how to mock the LocationManager was big enough to (I think) break one of the common rules in TDD -- a test implementation should not consume too much time.

So I figured, would it be best (and still within the TDD scope) to just test the contract (LocationDataSource) itself? Mocking dataSource and then replacing the test above with:

@Test
fun isProviderDisable_ShouldReturnFalse() {
    // Given
    `when`(dataSource.isLocationAvailable()).thenReturn(false)

    // When
    dataSource.isLocationAvailable().subscribe(observer)

    // Then
    observer.assertNoErrors()
    observer.assertValue(false)
}

This would (obviously) provide the same result without going through the trouble of mocking a LocationManager. But, I think this defeats the purpose of the test -- since it only focuses on the contract itself -- and not the actual class that uses it.

I still think that maybe the first practice is still the proper way. That initially, it just takes time to familiarize with the mocking of Android classes. But I would love to know what the experts on TDD think.

Solution

Working backwards... this looks a little weird:

// Given
`when`(dataSource.isLocationAvailable()).thenReturn(false)

// When
dataSource.isLocationAvailable().subscribe(observer)

You've got a mock(LocationDataSource) talking to a TestObserver. That test isn't completely without value, but if I'm not mistaken running tells you nothing new; if the code compiles, then the contract is satisfied.

In a language where you have reliable type checking, executed tests should have a test subject that is a production implementation. So in your second example, if observer were a test subject, that would be "fine".

I wouldn't pass that test in a code review -- unless there is spooky recursion at a distance going on, there's no reason to mock a method call that you are going to be making in the test itself.

// When
BehaviorSubject.createDefault(false).subscribe(testSubject);

the time I spent figuring out how to mock the LocationManager was big enough to (I think) break one of the common rules in TDD -- a test implementation should not consume too much time.

Right - your current design is fighting with you when you try to test it. That's a symptom; your job as the designer is to identify the problem.

In this case, the code you are trying to test it too tightly coupled to the LocationManager. It is common to create an interface/contract that you can hide a specific implementation behind. Sometimes this pattern is called a seam.

LocationManager::isProviderEnabled, from the outside, is just a function that takes a String and returns a boolean. So instead of writing your method in terms of the LocationManager, write it in terms of the capability that it will give you:

class LocationUtil(isProviderEnabled: (String) -> boolean ) : LocationDataSource {

  private var isAvailableSubject: BehaviorSubject<Boolean> = 
      BehaviorSubject.createDefault(isProviderEnabled(provider))

  override fun isAvailable(): Observable<Boolean> = locationSubject
}

In effect, we're trying to push the "hard to test" bits closer to the boundaries, where we'll rely on other techniques to address the risks.

Answered By - VoiceOfUnreason