From 3d525d4eb3bdafc4b31eeb737435dcb8e77bbf42 Mon Sep 17 00:00:00 2001 From: Paul Hawke Date: Sat, 23 Dec 2023 00:31:10 -0600 Subject: [PATCH] Removed butterknife from contributions list fragment (#5396) * Removed butterknife from contributions list fragment and overhauled its test * Suggested fix from stack overflow to remove duplicate class error during build --- .gitignore | 4 + app/build.gradle | 25 ++- .../ContributionsListFragment.java | 117 ++++++------- .../ContributionsListFragmentUnitTests.kt | 154 +++++------------- data-client/build.gradle | 10 +- 5 files changed, 126 insertions(+), 184 deletions(-) diff --git a/.gitignore b/.gitignore index 418e4c380..e54ea2551 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,7 @@ app/src/main/jniLibs #https://docs.opencv.org/3.3.0/ /libraries/opencv/javadoc/ captures/* + +# Test and other output +app/jacoco.exec +app/CommonsContributions \ No newline at end of file diff --git a/app/build.gradle b/app/build.gradle index 2c0b0d93c..6a197c811 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -75,20 +75,22 @@ dependencies { kapt "com.google.dagger:dagger-compiler:$DAGGER_VERSION" annotationProcessor "com.google.dagger:dagger-android-processor:$DAGGER_VERSION" - implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$KOTLIN_VERSION" implementation "org.jetbrains.kotlin:kotlin-reflect:$KOTLIN_VERSION" //Mocking testImplementation 'com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0' testImplementation 'org.mockito:mockito-inline:5.2.0' - testImplementation 'org.mockito:mockito-core:5.5.0' + testImplementation 'org.mockito:mockito-core:5.6.0' testImplementation "org.powermock:powermock-module-junit4:2.0.9" testImplementation "org.powermock:powermock-api-mockito2:2.0.9" // Unit testing testImplementation 'junit:junit:4.13.2' - testImplementation 'org.robolectric:robolectric:4.10.3' + testImplementation 'org.robolectric:robolectric:4.11.1' testImplementation 'androidx.test:core:1.5.0' + testImplementation "androidx.test:runner:1.5.2" + testImplementation 'androidx.test.ext:junit:1.1.5' + testImplementation "androidx.test:rules:1.5.0" testImplementation "com.squareup.okhttp3:mockwebserver:$OKHTTP_VERSION" testImplementation "com.jraska.livedata:testing-ktx:1.1.2" testImplementation "androidx.arch.core:core-testing:2.2.0" @@ -96,9 +98,9 @@ dependencies { testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:5.10.0" testImplementation 'com.facebook.soloader:soloader:0.10.5' testImplementation "org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.3" + debugImplementation("androidx.fragment:fragment-testing:1.6.2") // Android testing - androidTestImplementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$KOTLIN_VERSION" androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.0-alpha04' androidTestImplementation 'androidx.test.espresso:espresso-intents:3.4.0' androidTestImplementation 'androidx.test.espresso:espresso-contrib:3.5.0-alpha04' @@ -158,6 +160,15 @@ dependencies { kaptAndroidTest "androidx.databinding:databinding-compiler:8.0.2" implementation("io.github.coordinates2country:coordinates2country-android:1.3") { exclude group: 'com.google.android', module: 'android' } + + constraints { + implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.0") { + because("kotlin-stdlib-jdk7 is now a part of kotlin-stdlib") + } + implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0") { + because("kotlin-stdlib-jdk8 is now a part of kotlin-stdlib") + } + } } task disableAnimations(type: Exec) { @@ -206,8 +217,10 @@ android { testOptions { animationsDisabled true - unitTests.returnDefaultValues = true - unitTests.includeAndroidResources = true + unitTests { + returnDefaultValues = true + includeAndroidResources = true + } unitTests.all { jvmArgs '-noverify' diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java index 6fed83a5d..20435da34 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java @@ -24,6 +24,7 @@ import androidx.activity.result.ActivityResultLauncher; import androidx.activity.result.contract.ActivityResultContracts; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import androidx.appcompat.widget.AppCompatTextView; import androidx.fragment.app.FragmentManager; import androidx.recyclerview.widget.GridLayoutManager; @@ -32,15 +33,13 @@ import androidx.recyclerview.widget.RecyclerView.AdapterDataObserver; import androidx.recyclerview.widget.RecyclerView.ItemAnimator; import androidx.recyclerview.widget.RecyclerView.OnItemTouchListener; import androidx.recyclerview.widget.SimpleItemAnimator; -import butterknife.BindView; -import butterknife.ButterKnife; -import butterknife.OnClick; import com.google.android.material.floatingactionbutton.FloatingActionButton; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.Utils; import fr.free.nrw.commons.auth.SessionManager; +import fr.free.nrw.commons.databinding.FragmentContributionsListBinding; import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.media.MediaClient; import fr.free.nrw.commons.profile.ProfileActivity; @@ -66,61 +65,43 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl private static final String RV_STATE = "rv_scroll_state"; - @BindView(R.id.contributionsList) - RecyclerView rvContributionsList; - @BindView(R.id.loadingContributionsProgressBar) - ProgressBar progressBar; - @BindView(R.id.fab_plus) - FloatingActionButton fabPlus; - @BindView(R.id.fab_camera) - FloatingActionButton fabCamera; - @BindView(R.id.fab_gallery) - FloatingActionButton fabGallery; - @BindView(R.id.noContributionsYet) - TextView noContributionsYet; - @BindView(R.id.fab_layout) - LinearLayout fab_layout; - @BindView(R.id.fab_custom_gallery) - FloatingActionButton fabCustomGallery; - @Inject SystemThemeUtils systemThemeUtils; - @BindView(R.id.tv_contributions_of_user) - AppCompatTextView tvContributionsOfUser; - @Inject ContributionController controller; @Inject MediaClient mediaClient; - @Named(NAMED_LANGUAGE_WIKI_PEDIA_WIKI_SITE) @Inject WikiSite languageWikipediaSite; - @Inject ContributionsListPresenter contributionsListPresenter; - @Inject SessionManager sessionManager; + private FragmentContributionsListBinding binding; private Animation fab_close; private Animation fab_open; private Animation rotate_forward; private Animation rotate_backward; - - private boolean isFabOpen; - private ContributionsListAdapter adapter; + @VisibleForTesting + protected RecyclerView rvContributionsList; + + @VisibleForTesting + protected ContributionsListAdapter adapter; @Nullable - private Callback callback; + @VisibleForTesting + protected Callback callback; private final int SPAN_COUNT_LANDSCAPE = 3; private final int SPAN_COUNT_PORTRAIT = 1; private int contributionsSize; - String userName; + private String userName; + private ActivityResultLauncher inAppCameraLocationPermissionLauncher = registerForActivityResult(new ActivityResultContracts.RequestMultiplePermissions(), new ActivityResultCallback>() { @Override public void onActivityResult(Map result) { @@ -162,21 +143,32 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl public View onCreateView( final LayoutInflater inflater, @Nullable final ViewGroup container, @Nullable final Bundle savedInstanceState) { - final View view = inflater.inflate(R.layout.fragment_contributions_list, container, false); - ButterKnife.bind(this, view); + binding = FragmentContributionsListBinding.inflate( + inflater, container, false + ); + rvContributionsList = binding.contributionsList; + contributionsListPresenter.onAttachView(this); + binding.fabCustomGallery.setOnClickListener(v -> launchCustomSelector()); if (Objects.equals(sessionManager.getUserName(), userName)) { - tvContributionsOfUser.setVisibility(GONE); - fab_layout.setVisibility(VISIBLE); + binding.tvContributionsOfUser.setVisibility(GONE); + binding.fabLayout.setVisibility(VISIBLE); } else { - tvContributionsOfUser.setVisibility(VISIBLE); - tvContributionsOfUser.setText(getString(R.string.contributions_of_user, userName)); - fab_layout.setVisibility(GONE); + binding.tvContributionsOfUser.setVisibility(VISIBLE); + binding.tvContributionsOfUser.setText(getString(R.string.contributions_of_user, userName)); + binding.fabLayout.setVisibility(GONE); } initAdapter(); - return view; + + return binding.getRoot(); + } + + @Override + public void onDestroyView() { + binding = null; + super.onDestroyView(); } @Override @@ -309,7 +301,7 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl public void onConfigurationChanged(final Configuration newConfig) { super.onConfigurationChanged(newConfig); // check orientation - fab_layout.setOrientation(newConfig.orientation == Configuration.ORIENTATION_LANDSCAPE ? + binding.fabLayout.setOrientation(newConfig.orientation == Configuration.ORIENTATION_LANDSCAPE ? LinearLayout.HORIZONTAL : LinearLayout.VERTICAL); rvContributionsList .setLayoutManager( @@ -324,12 +316,12 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl } private void setListeners() { - fabPlus.setOnClickListener(view -> animateFAB(isFabOpen)); - fabCamera.setOnClickListener(view -> { + binding.fabPlus.setOnClickListener(view -> animateFAB(isFabOpen)); + binding.fabCamera.setOnClickListener(view -> { controller.initiateCameraPick(getActivity(), inAppCameraLocationPermissionLauncher); animateFAB(isFabOpen); }); - fabGallery.setOnClickListener(view -> { + binding.fabGallery.setOnClickListener(view -> { controller.initiateGalleryPick(getActivity(), true); animateFAB(isFabOpen); }); @@ -338,8 +330,7 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl /** * Launch Custom Selector. */ - @OnClick(R.id.fab_custom_gallery) - void launchCustomSelector() { + protected void launchCustomSelector() { controller.initiateCustomGalleryPickWithPermission(getActivity()); animateFAB(isFabOpen); } @@ -350,23 +341,23 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl private void animateFAB(final boolean isFabOpen) { this.isFabOpen = !isFabOpen; - if (fabPlus.isShown()) { + if (binding.fabPlus.isShown()) { if (isFabOpen) { - fabPlus.startAnimation(rotate_backward); - fabCamera.startAnimation(fab_close); - fabGallery.startAnimation(fab_close); - fabCustomGallery.startAnimation(fab_close); - fabCamera.hide(); - fabGallery.hide(); - fabCustomGallery.hide(); + binding.fabPlus.startAnimation(rotate_backward); + binding.fabCamera.startAnimation(fab_close); + binding.fabGallery.startAnimation(fab_close); + binding.fabCustomGallery.startAnimation(fab_close); + binding.fabCamera.hide(); + binding.fabGallery.hide(); + binding.fabCustomGallery.hide(); } else { - fabPlus.startAnimation(rotate_forward); - fabCamera.startAnimation(fab_open); - fabGallery.startAnimation(fab_open); - fabCustomGallery.startAnimation(fab_open); - fabCamera.show(); - fabGallery.show(); - fabCustomGallery.show(); + binding.fabPlus.startAnimation(rotate_forward); + binding.fabCamera.startAnimation(fab_open); + binding.fabGallery.startAnimation(fab_open); + binding.fabCustomGallery.startAnimation(fab_open); + binding.fabCamera.show(); + binding.fabGallery.show(); + binding.fabCustomGallery.show(); } this.isFabOpen = !isFabOpen; } @@ -377,7 +368,7 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl */ @Override public void showWelcomeTip(final boolean shouldShow) { - noContributionsYet.setVisibility(shouldShow ? VISIBLE : GONE); + binding.noContributionsYet.setVisibility(shouldShow ? VISIBLE : GONE); } /** @@ -387,12 +378,12 @@ public class ContributionsListFragment extends CommonsDaggerSupportFragment impl */ @Override public void showProgress(final boolean shouldShow) { - progressBar.setVisibility(shouldShow ? VISIBLE : GONE); + binding.loadingContributionsProgressBar.setVisibility(shouldShow ? VISIBLE : GONE); } @Override public void showNoContributionsUI(final boolean shouldShow) { - noContributionsYet.setVisibility(shouldShow ? VISIBLE : GONE); + binding.noContributionsYet.setVisibility(shouldShow ? VISIBLE : GONE); } @Override diff --git a/app/src/test/kotlin/fr/free/nrw/commons/contributions/ContributionsListFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/contributions/ContributionsListFragmentUnitTests.kt index 7d50e083e..936b0fd94 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/contributions/ContributionsListFragmentUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/contributions/ContributionsListFragmentUnitTests.kt @@ -1,18 +1,11 @@ package fr.free.nrw.commons.contributions -import android.content.Context import android.content.res.Configuration -import android.os.Bundle import android.os.Looper -import android.view.LayoutInflater -import android.widget.LinearLayout -import android.widget.ProgressBar -import android.widget.TextView -import androidx.fragment.app.FragmentManager -import androidx.fragment.app.FragmentTransaction -import androidx.recyclerview.widget.GridLayoutManager -import androidx.recyclerview.widget.RecyclerView -import androidx.test.core.app.ApplicationProvider +import androidx.fragment.app.testing.FragmentScenario +import androidx.fragment.app.testing.launchFragmentInContainer +import androidx.lifecycle.Lifecycle +import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.android.material.floatingactionbutton.FloatingActionButton import fr.free.nrw.commons.Media import fr.free.nrw.commons.TestAppAdapter @@ -23,113 +16,48 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyInt -import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.Mockito.`when` -import org.mockito.MockitoAnnotations -import org.powermock.reflect.Whitebox -import org.robolectric.Robolectric -import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import org.wikipedia.AppAdapter import java.lang.reflect.Method +import fr.free.nrw.commons.R +import org.mockito.Mockito.mock -@RunWith(RobolectricTestRunner::class) +@RunWith(AndroidJUnit4::class) @Config(sdk = [21], application = TestCommonsApplication::class) @LooperMode(LooperMode.Mode.PAUSED) class ContributionsListFragmentUnitTests { - private lateinit var activity: MainActivity + private lateinit var scenario: FragmentScenario private lateinit var fragment: ContributionsListFragment - private lateinit var context: Context - private lateinit var layoutInflater: LayoutInflater - @Mock - private lateinit var savedInstanceState: Bundle - - @Mock - private lateinit var rvContributionsList: RecyclerView - - @Mock - private lateinit var adapter: ContributionsListAdapter - - @Mock - private lateinit var contribution: Contribution - - @Mock - private lateinit var media: Media - - @Mock - private lateinit var wikidataPlace: WikidataPlace - - @Mock - private lateinit var callback: ContributionsListFragment.Callback - - @Mock - private lateinit var layoutManager: RecyclerView.LayoutManager - - @Mock - private lateinit var gridLayoutManager: GridLayoutManager - - @Mock - private lateinit var noContributionsYet: TextView - - @Mock - private lateinit var progressBar: ProgressBar - - @Mock - private lateinit var fabPlus: FloatingActionButton - - @Mock - private lateinit var fabCamera: FloatingActionButton - - @Mock - private lateinit var fabGallery: FloatingActionButton - - @Mock - private lateinit var fabCustomGallery: FloatingActionButton - - @Mock - private lateinit var newConfig: Configuration - - @Mock - private lateinit var fabLayout: LinearLayout - - @Mock - private lateinit var contributionsListPresenter: ContributionsListPresenter + private val adapter: ContributionsListAdapter = mock() + private val contribution: Contribution = mock() + private val media: Media = mock() + private val wikidataPlace: WikidataPlace = mock() @Before fun setUp() { - MockitoAnnotations.initMocks(this) AppAdapter.set(TestAppAdapter()) - context = ApplicationProvider.getApplicationContext() - activity = Robolectric.buildActivity(MainActivity::class.java).create().get() - layoutInflater = LayoutInflater.from(activity) + scenario = launchFragmentInContainer( + initialState = Lifecycle.State.RESUMED, + themeResId = R.style.LightAppTheme + ) { + ContributionsListFragment().apply { + contributionsListPresenter = mock() + callback = mock() + }.also { + fragment = it + } + } - fragment = ContributionsListFragment() - val fragmentManager: FragmentManager = activity.supportFragmentManager - val fragmentTransaction: FragmentTransaction = fragmentManager.beginTransaction() - fragmentTransaction.add(fragment, null) - fragmentTransaction.commit() - - Whitebox.setInternalState(fragment, "rvContributionsList", rvContributionsList) - Whitebox.setInternalState(fragment, "adapter", adapter) - Whitebox.setInternalState(fragment, "callback", callback) - Whitebox.setInternalState(fragment, "noContributionsYet", noContributionsYet) - Whitebox.setInternalState(fragment, "progressBar", progressBar) - Whitebox.setInternalState(fragment, "fabPlus", fabPlus) - Whitebox.setInternalState(fragment, "fabCamera", fabCamera) - Whitebox.setInternalState(fragment, "fabGallery", fabGallery) - Whitebox.setInternalState(fragment, "fabCustomGallery", fabCustomGallery) - Whitebox.setInternalState(fragment, "fab_layout", fabLayout) - Whitebox.setInternalState( - fragment, - "contributionsListPresenter", - contributionsListPresenter - ) + scenario.onFragment { + it.adapter = adapter + } } @Test @@ -139,13 +67,6 @@ class ContributionsListFragmentUnitTests { Assert.assertNotNull(fragment) } - @Test - @Throws(Exception::class) - fun testOnCreateView() { - Shadows.shadowOf(Looper.getMainLooper()).idle() - fragment.onCreateView(layoutInflater, null, savedInstanceState) - } - @Test @Throws(Exception::class) fun testOnDetach() { @@ -165,8 +86,9 @@ class ContributionsListFragmentUnitTests { @Throws(Exception::class) fun testOnScrollToTop() { Shadows.shadowOf(Looper.getMainLooper()).idle() + fragment.rvContributionsList = mock() fragment.scrollToTop() - verify(rvContributionsList).smoothScrollToPosition(0) + verify(fragment.rvContributionsList).smoothScrollToPosition(0) } @Test @@ -261,16 +183,14 @@ class ContributionsListFragmentUnitTests { @Throws(Exception::class) fun testOnViewStateRestored() { Shadows.shadowOf(Looper.getMainLooper()).idle() - `when`(rvContributionsList.layoutManager).thenReturn(layoutManager) - fragment.onViewStateRestored(savedInstanceState) + fragment.onViewStateRestored(mock()) } @Test @Throws(Exception::class) fun testOnSaveInstanceState() { Shadows.shadowOf(Looper.getMainLooper()).idle() - `when`(rvContributionsList.layoutManager).thenReturn(gridLayoutManager) - fragment.onSaveInstanceState(savedInstanceState) + fragment.onSaveInstanceState(mock()) } @Test @@ -298,7 +218,9 @@ class ContributionsListFragmentUnitTests { @Throws(Exception::class) fun testAnimateFAB() { Shadows.shadowOf(Looper.getMainLooper()).idle() - `when`(fabPlus.isShown).thenReturn(false) + scenario.onFragment { + it.requireView().findViewById(R.id.fab_plus).hide() + } val method: Method = ContributionsListFragment::class.java.getDeclaredMethod( "animateFAB", Boolean::class.java @@ -311,7 +233,9 @@ class ContributionsListFragmentUnitTests { @Throws(Exception::class) fun testAnimateFABCaseShownAndOpen() { Shadows.shadowOf(Looper.getMainLooper()).idle() - `when`(fabPlus.isShown).thenReturn(true) + scenario.onFragment { + it.requireView().findViewById(R.id.fab_plus).show() + } val method: Method = ContributionsListFragment::class.java.getDeclaredMethod( "animateFAB", Boolean::class.java @@ -324,7 +248,9 @@ class ContributionsListFragmentUnitTests { @Throws(Exception::class) fun testAnimateFABCaseShownAndClose() { Shadows.shadowOf(Looper.getMainLooper()).idle() - `when`(fabPlus.isShown).thenReturn(true) + scenario.onFragment { + it.requireView().findViewById(R.id.fab_plus).show() + } val method: Method = ContributionsListFragment::class.java.getDeclaredMethod( "animateFAB", Boolean::class.java @@ -359,8 +285,8 @@ class ContributionsListFragmentUnitTests { @Throws(Exception::class) fun testOnConfigurationChanged() { Shadows.shadowOf(Looper.getMainLooper()).idle() + val newConfig: Configuration = mock() newConfig.orientation = Configuration.ORIENTATION_LANDSCAPE fragment.onConfigurationChanged(newConfig) } - } \ No newline at end of file diff --git a/data-client/build.gradle b/data-client/build.gradle index e8c732dc5..3b2a33cd1 100644 --- a/data-client/build.gradle +++ b/data-client/build.gradle @@ -78,7 +78,6 @@ dependencies { implementation "io.reactivex.rxjava2:rxjava:2.2.3" implementation "io.reactivex.rxjava2:rxandroid:2.1.0" implementation 'org.apache.commons:commons-lang3:3.8.1' - implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version" androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.1' @@ -88,6 +87,15 @@ dependencies { testImplementation 'org.hamcrest:hamcrest-junit:2.0.0.0' testImplementation "com.squareup.okhttp3:mockwebserver:4.10.0" testImplementation "commons-io:commons-io:2.6" + + constraints { + implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.0") { + because("kotlin-stdlib-jdk7 is now a part of kotlin-stdlib") + } + implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0") { + because("kotlin-stdlib-jdk8 is now a part of kotlin-stdlib") + } + } } repositories {