From bbe2e117a01345b2282e45a2575edf545c60058a Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Fri, 21 Nov 2014 17:40:06 -0500 Subject: [PATCH 1/7] Add a stacktrace to testing --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 95cc636..76c499a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: android script: - - ./gradlew test + - ./gradlew test --stacktrace after_success: - - ./gradlew jacocoTestReport coveralls --stacktrace \ No newline at end of file + - ./gradlew jacocoTestReport coveralls --stacktrace From c94068721a1ee35f12a55ea759fe12164549f3bb Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Fri, 21 Nov 2014 18:01:35 -0500 Subject: [PATCH 2/7] Add the actual stacktrace --- .travis.yml | 4 ++-- app-test/build.gradle | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 76c499a..4be7cf3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: android script: - - ./gradlew test --stacktrace + - ./gradlew test after_success: - - ./gradlew jacocoTestReport coveralls --stacktrace + - ./gradlew jacocoTestReport coveralls diff --git a/app-test/build.gradle b/app-test/build.gradle index 92f888b..7236ba3 100644 --- a/app-test/build.gradle +++ b/app-test/build.gradle @@ -22,6 +22,12 @@ repositories { } } +test { + testLogging { + exceptionFormat = 'full' + } +} + def androidModule = project(':app') def firstVariant = androidModule.android.applicationVariants.toList().first() From ff6d95b793ec19ce4083393f71143473aab806d5 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Fri, 21 Nov 2014 18:23:52 -0500 Subject: [PATCH 3/7] Use maven central over jCenter --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index dad0887..b679517 100644 --- a/build.gradle +++ b/build.gradle @@ -4,7 +4,7 @@ buildscript { ext.kotlin_version = '0.9.206' repositories { - jcenter() + mavenCentral() } dependencies { classpath 'com.android.tools.build:gradle:0.13.+' @@ -16,6 +16,6 @@ buildscript { allprojects { repositories { - jcenter() + mavenCentral() } } From cc3c8ea48628bcc149a493f08a98a2614b2956b9 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Fri, 21 Nov 2014 18:24:24 -0500 Subject: [PATCH 4/7] Fix Mockito interfering with Hamcrest --- app-test/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app-test/build.gradle b/app-test/build.gradle index 7236ba3..8bdbb9c 100644 --- a/app-test/build.gradle +++ b/app-test/build.gradle @@ -36,7 +36,7 @@ dependencies { testCompile 'junit:junit:4.+' testCompile 'org.robolectric:robolectric:+' - testCompile 'org.mockito:mockito-all:+' + testCompile 'org.mockito:mockito-core:+' testCompile 'com.jayway.awaitility:awaitility:+' testCompile 'org.jetbrains.spek:spek:+' From 18d3620da3b05e564fc108af547543bffbb1bac8 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sat, 22 Nov 2014 16:28:05 -0500 Subject: [PATCH 5/7] Refactor a bit to make testing easier --- .../activity/downloader/BookListFragment.java | 77 +++++++------------ .../downloader/manager/RefreshManager.kt | 4 +- 2 files changed, 30 insertions(+), 51 deletions(-) diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java index fdc2023..925ae5d 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java @@ -2,7 +2,6 @@ package org.bspeice.minimalbible.activity.downloader; import android.app.Activity; import android.app.AlertDialog; -import android.app.ProgressDialog; import android.content.DialogInterface; import android.os.Bundle; import android.view.LayoutInflater; @@ -11,7 +10,6 @@ import android.view.ViewGroup; import android.widget.ArrayAdapter; import android.widget.ListView; import android.widget.Spinner; -import android.widget.SpinnerAdapter; import android.widget.Toast; import org.bspeice.minimalbible.Injector; @@ -45,9 +43,9 @@ public class BookListFragment extends BaseFragment { protected static final String ARG_BOOK_CATEGORY = "book_category"; @Inject - protected DownloadPrefs downloadPrefs; - protected ProgressDialog refreshDialog; - @Inject RefreshManager refreshManager; + DownloadPrefs downloadPrefs; + @Inject + RefreshManager refreshManager; @Inject List availableLanguages; @@ -56,7 +54,7 @@ public class BookListFragment extends BaseFragment { @InjectView(R.id.spn_available_languages) Spinner languagesSpinner; - private LayoutInflater inflater; + LayoutInflater inflater; /** * Returns a new instance of this fragment for the given section number. @@ -99,8 +97,8 @@ public class BookListFragment extends BaseFragment { * Trigger the functionality to display a list of modules. Prompts user if downloading * from the internet is allowable. */ - protected void displayModules() { - boolean dialogDisplayed = downloadPrefs.hasShownDownloadDialog(); + void displayModules() { + boolean dialogDisplayed = downloadPrefs.hasShownDownloadDialog(); if (!dialogDisplayed) { AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); @@ -111,45 +109,22 @@ public class BookListFragment extends BaseFragment { .setNegativeButton("No", dialogListener) .setCancelable(false).show(); } else { - refreshModules(); - } + displayLanguageSpinner(); + } } - /** - * Do the work of refreshing modules (download manager handles using cached copy vs. actual - * refresh), and then displaying them when ready. - */ - private void refreshModules() { - // Check if the downloadManager has already refreshed everything - if (!refreshManager.getRefreshComplete().get()) { - // downloadManager is in progress of refreshing - refreshDialog = new ProgressDialog(getActivity()); - refreshDialog.setMessage("Refreshing available modules..."); - refreshDialog.setCancelable(false); - refreshDialog.show(); - } - - languagesSpinner.setAdapter(getLocaleSpinner()); - if (BookListFragment.this.getActivity() != null) { - // On a screen rotate, getActivity() will be null. But, the activity - // will already have been set up correctly, so we don't need to worry - // about it. - // If not null, we need to set it up now. - setInsetsSpinner(BookListFragment.this.getActivity(), languagesSpinner); - } - if (refreshDialog != null) { - refreshDialog.cancel(); - } - } - - @SuppressWarnings("ConstantConditions") - // getAvailableLanguagesList() will not return null - SpinnerAdapter getLocaleSpinner() { + void displayLanguageSpinner() { ArrayAdapter adapter = new ArrayAdapter(this.getActivity(), android.R.layout.simple_spinner_item, availableLanguages.toArray()); adapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); - return adapter; + languagesSpinner.setAdapter(adapter); + + if (BookListFragment.this.getActivity() != null) { + // On a screen rotate, getActivity() will be null, but the activity + // will already have been set up. If not null, we need to set it up now. + setInsetsSpinner(BookListFragment.this.getActivity(), languagesSpinner); + } } @SuppressWarnings("unused") @@ -207,23 +182,27 @@ public class BookListFragment extends BaseFragment { downloadPrefs.hasEnabledDownload(true); // And warn them that it has been enabled in the future. - Toast.makeText(getActivity(), - "Downloading now enabled. Disable in settings.", - Toast.LENGTH_SHORT).show(); - refreshModules(); + showToast("Downloading now enabled. Disable in settings"); + displayModules(); break; // case DialogInterface.BUTTON_NEGATIVE: default: // Clicked to not download - Permanently disable downloading downloadPrefs.hasEnabledDownload(false); - Toast.makeText(getActivity(), - "Disabling downloading. Re-enable it in settings.", - Toast.LENGTH_SHORT).show(); - refreshModules(); + showToast("Disabling downloading. Re-enable it in settings."); + shutdown(); break; } } + + void showToast(String text) { + Toast.makeText(getActivity(), text, Toast.LENGTH_SHORT).show(); + } + + void shutdown() { + getActivity().finish(); + } } } \ No newline at end of file diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/RefreshManager.kt b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/RefreshManager.kt index 4d206f0..94ea0ec 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/RefreshManager.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/RefreshManager.kt @@ -48,9 +48,9 @@ class RefreshManager(val installers: Collection, val fifteenDaysAgo = Calendar.getInstance().getTime().getTime() - 1296000 - fun doReload(enabledDownload: Boolean, lastUpdated: Long, + fun doReload(downloadEnabled: Boolean, lastUpdated: Long, networkState: Int? = ConnectivityManager.TYPE_DUMMY): Boolean = - if (!enabledDownload || networkState != ConnectivityManager.TYPE_WIFI) + if (!downloadEnabled || networkState != ConnectivityManager.TYPE_WIFI) false else if (lastUpdated < fifteenDaysAgo) true From 05d2d006e457718723fbe065a32b4ee443b9cbdf Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sat, 22 Nov 2014 17:07:58 -0500 Subject: [PATCH 6/7] Boundary refactor, and add some tests --- .../downloader/BookListFragmentTest.kt | 52 ++++++++++++++ .../downloader/BookListFragmentTest.java | 70 ------------------- .../activity/downloader/BookListFragment.java | 30 ++++---- 3 files changed, 70 insertions(+), 82 deletions(-) create mode 100644 app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt delete mode 100644 app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/BookListFragmentTest.java diff --git a/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt b/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt new file mode 100644 index 0000000..c42f3a0 --- /dev/null +++ b/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt @@ -0,0 +1,52 @@ +package org.bspeice.minimalbible.activity.downloader + +import org.jetbrains.spek.api.Spek +import kotlin.test.assertTrue + +/** + * Created by bspeice on 11/22/14. + */ + +class BookListFragmentSpek : Spek() {{ + + given("A BookListFragment with showDialog() mocked out") { + class TestableFragment : BookListFragment() { + var condition = false + + override fun showDialog() { + condition = true + } + } + + val fragment = TestableFragment() + + on("attempting to display modules with the dialog not shown already") { + fragment.displayModules(false) + + it("should show the download dialog") { + assertTrue(fragment.condition) + } + } + } + + given("a BookListFragment with displayLanguageSpinner() mocked out") { + class TestableFragment : BookListFragment() { + var condition = false + + override fun displayLanguageSpinner() { + condition = true + } + } + + val fragment = TestableFragment() + + on("attempting to display modules with the dialog already shown") { + fragment.displayModules(true) + + it("should show the available languages spinner") { + assertTrue(fragment.condition) + } + } + } +} +} \ No newline at end of file diff --git a/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/BookListFragmentTest.java b/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/BookListFragmentTest.java deleted file mode 100644 index 0a74541..0000000 --- a/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/BookListFragmentTest.java +++ /dev/null @@ -1,70 +0,0 @@ -package org.bspeice.minimalbible.test.activity.downloader; - -import org.bspeice.minimalbible.MBTestCase; -import org.bspeice.minimalbible.activity.downloader.BookListFragment; -import org.crosswire.common.util.Language; -import org.crosswire.jsword.book.Book; -import org.crosswire.jsword.book.BookCategory; -import org.mockito.Mockito; - -import rx.Observable; -import rx.functions.Action1; - -import static org.mockito.Mockito.when; - -public class BookListFragmentTest extends MBTestCase { - - public void testBooksByLanguage() throws Exception { - BookCategory bibleCategory = BookCategory.BIBLE; - BookCategory dictionaryCategory = BookCategory.DICTIONARY; - Language russianLanguage = new Language("ru"); - Language englishLanguage = new Language("en"); - - final Book russianBible = Mockito.mock(Book.class); - when(russianBible.getBookCategory()).thenReturn(bibleCategory); - when(russianBible.getLanguage()).thenReturn(russianLanguage); - - final Book englishBible = Mockito.mock(Book.class); - when(englishBible.getBookCategory()).thenReturn(bibleCategory); - when(englishBible.getLanguage()).thenReturn(englishLanguage); - - final Book englishDictionary = Mockito.mock(Book.class); - when(englishDictionary.getBookCategory()).thenReturn(dictionaryCategory); - when(englishDictionary.getLanguage()).thenReturn(englishLanguage); - - Observable mockBooks = Observable.just(russianBible, englishBible, - englishDictionary); - - // Since we're not testing lifecycle here, don't worry about newInstance() - TestableBookListFragment fragment = new TestableBookListFragment(); - - fragment.booksByLanguage(mockBooks, englishLanguage, bibleCategory) - .subscribe(new Action1() { - @Override - public void call(Book book) { - assertEquals(englishBible, book); - } - }); - fragment.booksByLanguage(mockBooks, russianLanguage, bibleCategory) - .subscribe(new Action1() { - @Override - public void call(Book book) { - assertEquals(russianBible, book); - } - }); - fragment.booksByLanguage(mockBooks, englishLanguage, dictionaryCategory) - .subscribe(new Action1() { - @Override - public void call(Book book) { - assertEquals(englishDictionary, book); - } - }); - } - - public static class TestableBookListFragment extends BookListFragment { - @Override - public Observable booksByLanguage(Observable books, Language language, BookCategory category) { - return super.booksByLanguage(books, language, category); - } - } -} \ No newline at end of file diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java index 925ae5d..905ba58 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java @@ -93,26 +93,32 @@ public class BookListFragment extends BaseFragment { .getString(ARG_BOOK_CATEGORY)); } + void displayModules() { + displayModules(downloadPrefs.hasShownDownloadDialog()); + } + /** * Trigger the functionality to display a list of modules. Prompts user if downloading * from the internet is allowable. */ - void displayModules() { - boolean dialogDisplayed = downloadPrefs.hasShownDownloadDialog(); - - if (!dialogDisplayed) { - AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); - DownloadDialogListener dialogListener = new DownloadDialogListener(); - builder.setMessage( - "About to contact servers to download content. Continue?") - .setPositiveButton("Yes", dialogListener) - .setNegativeButton("No", dialogListener) - .setCancelable(false).show(); - } else { + void displayModules(boolean dialogDisplayed) { + if (!dialogDisplayed) { + showDialog(); + } else { displayLanguageSpinner(); } } + void showDialog() { + AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); + DownloadDialogListener dialogListener = new DownloadDialogListener(); + builder.setMessage( + "About to contact servers to download content. Continue?") + .setPositiveButton("Yes", dialogListener) + .setNegativeButton("No", dialogListener) + .setCancelable(false).show(); + } + void displayLanguageSpinner() { ArrayAdapter adapter = new ArrayAdapter(this.getActivity(), android.R.layout.simple_spinner_item, From 8d65327853ee5ee383a98e2ec41e7a7d6600edd9 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sat, 22 Nov 2014 17:31:53 -0500 Subject: [PATCH 7/7] More boundary refactoring and testing Only testing branch logic is so nice... --- .../downloader/BookListFragmentTest.kt | 44 ++++++++++++++--- .../activity/downloader/BookListFragment.java | 48 ++++++++++++------- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt b/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt index c42f3a0..b2aa8e4 100644 --- a/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt +++ b/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/BookListFragmentTest.kt @@ -2,6 +2,7 @@ package org.bspeice.minimalbible.activity.downloader import org.jetbrains.spek.api.Spek import kotlin.test.assertTrue +import android.content.DialogInterface /** * Created by bspeice on 11/22/14. @@ -10,16 +11,13 @@ import kotlin.test.assertTrue class BookListFragmentSpek : Spek() {{ given("A BookListFragment with showDialog() mocked out") { - class TestableFragment : BookListFragment() { + val fragment = object : BookListFragment() { var condition = false - override fun showDialog() { condition = true } } - val fragment = TestableFragment() - on("attempting to display modules with the dialog not shown already") { fragment.displayModules(false) @@ -30,7 +28,7 @@ class BookListFragmentSpek : Spek() {{ } given("a BookListFragment with displayLanguageSpinner() mocked out") { - class TestableFragment : BookListFragment() { + val fragment = object : BookListFragment() { var condition = false override fun displayLanguageSpinner() { @@ -38,8 +36,6 @@ class BookListFragmentSpek : Spek() {{ } } - val fragment = TestableFragment() - on("attempting to display modules with the dialog already shown") { fragment.displayModules(true) @@ -48,5 +44,39 @@ class BookListFragmentSpek : Spek() {{ } } } + + given("a DownloadDialogListener with with buttonPositive() mocked out") { + val listener = object : BookListFragment.DownloadDialogListener(null, null) { + var condition = false + override fun buttonPositive() { + condition = true + } + } + + on("handling a positive button press") { + listener.handleButton(DialogInterface.BUTTON_POSITIVE) + + it("should call the proper handler") { + assertTrue(listener.condition) + } + } + } + + given("A DownloadDialogListener with buttonNegative() mocked out") { + val listener = object : BookListFragment.DownloadDialogListener(null, null) { + var condition = false + override fun buttonNegative() { + condition = true + } + } + + on("handling a negative button press") { + listener.handleButton(DialogInterface.BUTTON_NEGATIVE) + + it("should call the proper handler") { + assertTrue(listener.condition) + } + } + } } } \ No newline at end of file diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java index 905ba58..21fc527 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookListFragment.java @@ -111,7 +111,7 @@ public class BookListFragment extends BaseFragment { void showDialog() { AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); - DownloadDialogListener dialogListener = new DownloadDialogListener(); + DownloadDialogListener dialogListener = new DownloadDialogListener(this, downloadPrefs); builder.setMessage( "About to contact servers to download content. Continue?") .setPositiveButton("Yes", dialogListener) @@ -176,39 +176,53 @@ public class BookListFragment extends BaseFragment { }); } - private class DownloadDialogListener implements + static class DownloadDialogListener implements DialogInterface.OnClickListener { + BookListFragment fragment; + DownloadPrefs downloadPrefs; + + DownloadDialogListener(BookListFragment fragment, DownloadPrefs downloadPrefs) { + this.fragment = fragment; + this.downloadPrefs = downloadPrefs; + } + @Override public void onClick(@NotNull DialogInterface dialog, int which) { downloadPrefs.hasShownDownloadDialog(true); + handleButton(which); + } + void handleButton(int which) { switch (which) { case DialogInterface.BUTTON_POSITIVE: - // Clicked ready to continue - allow downloading in the future - downloadPrefs.hasEnabledDownload(true); - - // And warn them that it has been enabled in the future. - showToast("Downloading now enabled. Disable in settings"); - displayModules(); + buttonPositive(); break; // case DialogInterface.BUTTON_NEGATIVE: default: - // Clicked to not download - Permanently disable downloading - downloadPrefs.hasEnabledDownload(false); - showToast("Disabling downloading. Re-enable it in settings."); - shutdown(); + buttonNegative(); break; } } - void showToast(String text) { - Toast.makeText(getActivity(), text, Toast.LENGTH_SHORT).show(); + void buttonPositive() { + // Clicked ready to continue - allow downloading in the future + downloadPrefs.hasEnabledDownload(true); + + // And warn them that it has been enabled in the future. + showToast("Downloading now enabled. Disable in settings"); + fragment.displayModules(); } - void shutdown() { - getActivity().finish(); + void buttonNegative() { + // Clicked to not download - Permanently disable downloading + downloadPrefs.hasEnabledDownload(false); + showToast("Disabling downloading. Re-enable it in settings."); + fragment.getActivity().finish(); + } + + void showToast(String text) { + Toast.makeText(fragment.getActivity(), text, Toast.LENGTH_SHORT).show(); } } - } \ No newline at end of file