Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test case for BasePresenter #54

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

russhwolf
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Feb 6, 2018

55% (+55.37%) vs master 0%

Copy link
Contributor

@gdaniels gdaniels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better with Mockito.spy() -

package io.intrepid.skeleton.base;

import android.support.annotation.NonNull;

import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;

import io.intrepid.skeleton.testutils.PresenterTestBase;
import io.reactivex.Observable;
import io.reactivex.disposables.Disposable;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

public class BasePresenterTest extends PresenterTestBase<BasePresenter<BaseContract.View>> {
    @Mock
    private BaseContract.View view;

    private TestPresenter spyPresenter;

    private Disposable disposable;

    @Before
    public void setUp() {
        spyPresenter = Mockito.spy(new TestPresenter(view, testConfiguration));
        presenter = spyPresenter;
    }

    @Test
    public void bind_onlyOnce() throws Exception {
        presenter.bindView(view);
        presenter.bindView(view);
        assertEquals(view, presenter.view);
        verify(spyPresenter, times(1)).onViewBound();
    }

    @Test
    public void unbind_onlyOnce() throws Exception {
        presenter.unbindView();
        verify(spyPresenter, never()).onViewUnbound();

        presenter.bindView(view);
        presenter.unbindView();
        presenter.unbindView();
        verify(spyPresenter, times(1)).onViewUnbound();
    }

    @Test
    public void unbind_clearsDisposables() {
        presenter.bindView(view);
        assertFalse(disposable.isDisposed());

        presenter.unbindView();
        assertTrue(disposable.isDisposed());
    }

    private class TestPresenter extends BasePresenter<BaseContract.View> {

        TestPresenter(@NonNull BaseContract.View view,
                      @NonNull PresenterConfiguration configuration) {
            super(view, configuration);
        }

        @Override
        protected void onViewBound() {
            disposable = Observable.never().subscribe();
            disposables.add(disposable);
        }
    }

}

WDYT?

import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

public class BasePresenterTest extends PresenterTestBase<BasePresenter<BaseContract.View>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No real comment here, but that is quite a line of source code there. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming is hard 😄

@Test
public void unbind_onlyOnce() throws Exception {
presenter.unbindView();
verify(onUnbind, never()).run();
Copy link
Contributor

@gdaniels gdaniels Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, I'd actually split this into two tests, presenterWithoutViewDoesNotCallOnViewUnbound() (or whatever) and unbind_onlyOnce().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good call. I was bouncing around a bit in how I split these up since there's a bit of duplication due to lifecycle necessity, but this can definitely be split in two.

@russhwolf
Copy link
Contributor Author

To recap some in-person conversation RE Glen's point above, I recently had some issues on Motiv (cc @streetsofboston) where we were having issues with breakpoints in our tests when using spy(), so I've been wary of it recently (it's also somewhat discouraged in the Mockito documentation). Looking back at it, it seems that was specifically due to spying on anonymous classes which isn't what's happening here, so I'll do that refactor.

@ghost
Copy link

ghost commented Feb 7, 2018

56% (+56.2%) vs master 0%

@russhwolf
Copy link
Contributor Author

Heh, using spy() upped the coverage by one line, since we call into the real BasePresenter.onViewUnbound() instead of overriding it in TestPresenter. Should I increase it further by adding no-op tests that call into the rest of the empty default BasePresenter methods? They're all empty so there's nothing to test except that it doesn't crash.

@russhwolf
Copy link
Contributor Author

@gdaniels This has been sitting for a while. Any more thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants