4
\$\begingroup\$

I have an Android service which must offload a task to a different thread but schedule the response callback unto the calling thread. After much thinking I came up with this code:

public void testRequestingAScheduleUpdateShouldNotBlockAndDeliverItOnTheCallingThread() throws Exception {
    final StreamSchedule mockSchedule = new StreamSchedule(
        new StreamMetadata("TyPhone", "Tidal Waves 028"),
        new StreamMetadata[] { new StreamMetadata(null, "The Progmatic Radio Show 002 with Ashley Bonsall") },
        new StreamMetadata[] { new StreamMetadata("TEKNO", "Sound Escalation 065 with Xabi") }
    );

    final boolean[] threadBlocked = {true};
    LooperThread looperThread = new LooperThread();
    looperThread.start();

    StreamScheduleProvider mockScheduleProvider = mock(StreamScheduleProvider.class);
    when(mockScheduleProvider.getCurrentStreamSchedule()).then(invocation -> {
        threadBlocked[0] = Thread.currentThread().getId() == looperThread.getId();
        return mockSchedule;
    });
    DataAccessModule.scheduleProviderOverride = mockScheduleProvider;

    final long[] callingThreadId = {-1};

    ScheduleUpdateListener updateListener = mock(ScheduleUpdateListener.class);
    doAnswer(invocation -> {
        callingThreadId[0] = Thread.currentThread().getId();
        looperThread.stopLooper();
        return null;
    }).when(updateListener).onScheduleChanged(any(StreamSchedule.class));


    PlaybackServiceBinder serviceBinder = (PlaybackServiceBinder) bindService(getServiceIntent());
    serviceBinder.subscribeScheduleUpdates(updateListener);
    looperThread.getHandler().post(serviceBinder::updateSchedule);
    looperThread.join();

    verify(updateListener).onScheduleChanged(mockSchedule);
    assertFalse("The thread was blocked, this is unacceptable", threadBlocked[0]);
    assertEquals("The update callback was not called on the requesting thread", callingThreadId[0], looperThread.getId());
}

class LooperThread extends Thread {
    private Handler handler;

    @Override
    public void run() {
        Looper.prepare();
        handler = new Handler(Looper.myLooper());
        Looper.loop();
    }

    public Handler getHandler() {
        return handler;
    }

    public void stopLooper() {
        handler.getLooper().quit();
    }
}

It does work (the test fails if I simply execute the callback on the background thread or if I use no background thread at all) but I have this that intense itch I overlooked something that could make this more readable/elegant. Any ideas?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Disclaimer: I don't know Android. I've wrote two or three programs for it.

Also, I don't have your full code so can't be sure about some of the classes this code relies on.

  1. Test name has an "and", seems like two tests?
  2. Test throws Exception, do you have tests for that?
  3. You don't need to start test name with tests, unless you use JUnit 3.
  4. When LooperThread runs, it calls Looper.prepare() and Looper.loop(). What these do?
  5. LooperThread extends Thread, why not implement Runnable? You only implement run() then?
  6. Further reinforcement to point 1) - you have more than one assertion. If verify buckles, other assertions aren't tested. Is that fine?
  7. Why do you need a final boolean[] threadBlocked = {true}; over just a final boolean? I found no other reference than to that one element, via threadBlocked[0]?
  8. Is this test alone it it's class? If so, I'd extract mocking out of it. Readability / clarity / easier to spot things.
  9. Have you tried structuring the test into classic given - when - then or assess - act - assert?
\$\endgroup\$
1
  • \$\begingroup\$ I assume the final boolean [] is a hack-around to allow state-changes of a final object :/ \$\endgroup\$
    – Vogel612
    Commented Jan 7, 2016 at 21:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.