Improve Geo{Point,Trace,Shape}Activity tests by adding a FakeLocationClient

[zestyping]

Overview

In the real app, the LocationClient is the component that gets GPS location readings, which doesn't exist during unit testing. So, the current tests for these activities substitute a mock LocationClient—that means the tests poke at the internal methods of the activity and verify that it called specific methods on the LocationClient.

This PR changes the tests to use a fake LocationClient instead—that means we supply an object that actually behaves like a real LocationClient, operate the activity through its UI and external interface, and verify that the activity behaved properly.

Guidance to reviewers

Please note that this PR is based on #2776, not on master. To see the relevant diff, look only at the last commit: 63259c2

A good place to start is the FakeLocationClient—it's a simple implementation of the LocationClient interface, with a few extra methods (like receiveFix) to tell it how to behave. Then the tests should be fairly straightforward to understand. The changes in GeoPointMapActivity expose some of the string formatting functions so that the test isn't dependent on precise formatting.

What has been done to verify that this works as intended?

These tests all pass, and test coverage numbers (as measured by IntelliJ) are all greater than or equal to the previous coverage numbers for every class related to this change.

Why is this the best possible solution? Were any other approaches considered?

Fakes are usually better than mocks. Where feasible, using fakes leads to better tests; mocks should only be used as a last resort.

Mocking verifies the implementation. It only spot-checks that particular methods were called, so the test conditions are less realistic and only small bits of the code are exercised. Moreover, mock-style tests are fragile and will break if the implementation changes, because they depend both on the internal entry points invoked by the test as well as the specific methods invoked by the activity. Another way of saying this is that mocking violates encapsulation.

Faking verifies the behaviour. More of the unit-under-test gets exercised in a realistic way. When the test passes, it gives us more confidence that the unit-under-test actually behaved correctly. And if we refactor the unit-under-test in a way that doesn't change its public interface, the test doesn't break.

For an example, open the diff and have a look at the last dozen lines of GeoShapeActivityTest. The mocking-style test activityShouldShowErrorDialogOnClientError is on the left. The new test shouldShowErrorDialogIfLocationClientFails is on the right.

The mocking-style test on the left (a) invokes map.onClientStartFailure() and then (b) checks that the alert dialog is showing. But what does map.onClientStartFailure() do? Its body is just one line:

    @Override public void onClientStartFailure() {
        showGpsDisabledAlert();
    }

So the test mostly just confirms that this method contains this one line—which is obvious from looking at the code and hardly worth testing. An activity could have no interaction with LocationClient—no location functionality at all—and still pass this test!

In contrast, the new test on the right (a) sets up a FakeLocationClient that will simulate a failure, then (b) starts the activity, then (c) checks that the alert dialog is showing.

In order to pass this test, the activity must use the LocationClient in a way that would catch its failure in real life—in the course of starting up, the activity must at some point cause a LocationClient to be created, set up the appropriate listener to catch the failure event, and start the LocationClient so as to trigger the failure event.

How does this change affect users?

This PR only touches the unit tests; it has no user-visible effect.

Does this change require updates to documentation?

No.

Before submitting this PR, please make sure you have:

• run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[zestyping]

@kkrawczyk123 @mmarciniak90 Note this PR is affected by #2776; please see the note there.

[zestyping]

This pull request is now obsolete; it has been rolled into #2865.