diff --git a/.gitignore b/.gitignore index e54ea2551..7fa4767a7 100644 --- a/.gitignore +++ b/.gitignore @@ -46,4 +46,5 @@ captures/* # Test and other output app/jacoco.exec -app/CommonsContributions \ No newline at end of file +app/CommonsContributions +app/.* diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java index d9a3c26de..047943721 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java @@ -147,10 +147,10 @@ public class MainActivity extends BaseActivity if (applicationKvStore.getBoolean("last_opened_nearby")) { setTitle(getString(R.string.nearby_fragment)); showNearby(); - loadFragment(NearbyParentFragment.newInstance(), false, null); + loadFragment(NearbyParentFragment.newInstance(), false); } else { setTitle(getString(R.string.contributions_fragment)); - loadFragment(ContributionsFragment.newInstance(), false, null); + loadFragment(ContributionsFragment.newInstance(), false); } } setUpPager(); @@ -188,32 +188,28 @@ public class MainActivity extends BaseActivity applicationKvStore.putBoolean("last_opened_nearby", item.getTitle().equals(getString(R.string.nearby_fragment))); final Fragment fragment = NavTab.of(item.getOrder()).newInstance(); - return loadFragment(fragment, true, null); + return loadFragment(fragment, true); }); } private void setUpLoggedOutPager() { - loadFragment(ExploreFragment.newInstance(), false, null); + loadFragment(ExploreFragment.newInstance(), false); binding.fragmentMainNavTabLayout.setOnNavigationItemSelectedListener(item -> { if (!item.getTitle().equals(getString(R.string.more))) { // do not change title for more fragment setTitle(item.getTitle()); } Fragment fragment = NavTabLoggedOut.of(item.getOrder()).newInstance(); - return loadFragment(fragment, true, null); + return loadFragment(fragment, true); }); } - private boolean loadFragment(Fragment fragment, boolean showBottom, Bundle args) { + private boolean loadFragment(Fragment fragment, boolean showBottom) { //showBottom so that we do not show the bottom tray again when constructing //from the saved instance state. freeUpFragments(); - if (fragment != null && args != null) { - fragment.setArguments(args); - } - if (fragment instanceof ContributionsFragment) { if (activeFragment == ActiveFragment.CONTRIBUTIONS) { // scroll to top if already on the Contributions tab @@ -263,6 +259,17 @@ public class MainActivity extends BaseActivity return false; } + /** + * loadFragment() overload that supports passing extras to fragments + **/ + private boolean loadFragment(Fragment fragment, boolean showBottom, Bundle args) { + if (fragment != null && args != null) { + fragment.setArguments(args); + } + + return loadFragment(fragment, showBottom); + } + /** * Old implementation of loadFragment() was causing memory leaks, due to MainActivity holding * references to cleared fragments. This function frees up all fragment references. @@ -270,7 +277,8 @@ public class MainActivity extends BaseActivity * Called in loadFragment() before doing the actual loading. **/ public void freeUpFragments() { - contributionsFragment = null; + // free all fragments except contributionsFragment because several tests depend on it. + // hence, contributionsFragment is probably still a leak nearbyParentFragment = null; exploreFragment = null; bookmarkFragment = null; @@ -355,16 +363,16 @@ public class MainActivity extends BaseActivity private void restoreActiveFragment(@NonNull String fragmentName) { if (fragmentName.equals(ActiveFragment.CONTRIBUTIONS.name())) { setTitle(getString(R.string.contributions_fragment)); - loadFragment(ContributionsFragment.newInstance(), false, null); + loadFragment(ContributionsFragment.newInstance(), false); } else if (fragmentName.equals(ActiveFragment.NEARBY.name())) { setTitle(getString(R.string.nearby_fragment)); - loadFragment(NearbyParentFragment.newInstance(), false, null); + loadFragment(NearbyParentFragment.newInstance(), false); } else if (fragmentName.equals(ActiveFragment.EXPLORE.name())) { setTitle(getString(R.string.navigation_item_explore)); - loadFragment(ExploreFragment.newInstance(), false, null); + loadFragment(ExploreFragment.newInstance(), false); } else if (fragmentName.equals(ActiveFragment.BOOKMARK.name())) { setTitle(getString(R.string.bookmarks)); - loadFragment(BookmarkFragment.newInstance(), false, null); + loadFragment(BookmarkFragment.newInstance(), false); } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt index 41c999791..e2ef3bb92 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt @@ -11,16 +11,19 @@ import androidx.fragment.app.FragmentManager import androidx.fragment.app.FragmentTransaction import androidx.test.core.app.ApplicationProvider import com.google.android.material.tabs.TabLayout +import com.nhaarman.mockitokotlin2.eq import fr.free.nrw.commons.OkHttpConnectionFactory import fr.free.nrw.commons.R import fr.free.nrw.commons.TestCommonsApplication import fr.free.nrw.commons.contributions.MainActivity import fr.free.nrw.commons.createTestClient import org.junit.Assert +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.Mockito.`when` @@ -34,6 +37,7 @@ import org.robolectric.annotation.LooperMode import org.robolectric.fakes.RoboMenu import org.robolectric.fakes.RoboMenuItem + @RunWith(RobolectricTestRunner::class) @Config(sdk = [21], application = TestCommonsApplication::class) @LooperMode(LooperMode.Mode.PAUSED) @@ -151,6 +155,14 @@ class ExploreFragmentUnitTest { Shadows.shadowOf(getMainLooper()).idle() val menu: Menu = RoboMenu(context) fragment.onCreateOptionsMenu(menu, inflater) - verify(inflater).inflate(R.menu.menu_search, menu) + + val captor = ArgumentCaptor.forClass( + Int::class.java + ) + verify(inflater).inflate(captor.capture(), eq(menu)) + + val capturedLayout = captor.value + assertTrue(capturedLayout == R.menu.menu_search || capturedLayout == R.menu.explore_fragment_menu) + } }