Skip to content

Commit 841b86f

Browse files
Merge pull request #16378 from nextcloud/backport/16296/stable-3.35
[stable-3.35] Fixing timing issue with FileActivity's loading dialog show and dismiss
2 parents 60a0359 + c960583 commit 841b86f

4 files changed

Lines changed: 67 additions & 1 deletion

File tree

app/src/androidTest/java/com/nextcloud/client/FileDisplayActivityIT.kt

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/*
22
* Nextcloud - Android Client
33
*
4+
* SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info>
45
* SPDX-FileCopyrightText: 2019 Tobias Kaminsky <tobias@kaminsky.me>
56
* SPDX-FileCopyrightText: 2019 Nextcloud GmbH
67
* SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only
@@ -14,12 +15,14 @@ import androidx.test.espresso.Espresso.onView
1415
import androidx.test.espresso.IdlingRegistry
1516
import androidx.test.espresso.action.ViewActions.click
1617
import androidx.test.espresso.action.ViewActions.closeSoftKeyboard
18+
import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
1719
import androidx.test.espresso.assertion.ViewAssertions.matches
1820
import androidx.test.espresso.contrib.DrawerActions
1921
import androidx.test.espresso.contrib.NavigationViewActions
2022
import androidx.test.espresso.contrib.RecyclerViewActions
2123
import androidx.test.espresso.matcher.ViewMatchers
2224
import androidx.test.espresso.matcher.ViewMatchers.hasDescendant
25+
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
2326
import androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility
2427
import androidx.test.espresso.matcher.ViewMatchers.withId
2528
import androidx.test.espresso.matcher.ViewMatchers.withText
@@ -36,6 +39,7 @@ import com.owncloud.android.operations.CreateFolderOperation
3639
import com.owncloud.android.ui.activity.FileDisplayActivity
3740
import com.owncloud.android.ui.adapter.OCFileListItemViewHolder
3841
import com.owncloud.android.utils.EspressoIdlingResource
42+
import org.hamcrest.Matchers.allOf
3943
import org.junit.After
4044
import org.junit.Assert.assertEquals
4145
import org.junit.Assert.assertTrue
@@ -44,6 +48,7 @@ import org.junit.Rule
4448
import org.junit.Test
4549

4650
class FileDisplayActivityIT : AbstractOnServerIT() {
51+
4752
@Before
4853
fun registerIdlingResource() {
4954
IdlingRegistry.getInstance().register(EspressoIdlingResource.countingIdlingResource)
@@ -236,4 +241,53 @@ class FileDisplayActivityIT : AbstractOnServerIT() {
236241
}
237242
}
238243
}
244+
245+
@Test
246+
fun testShowAndDismissLoadingDialog() {
247+
launchActivity<FileDisplayActivity>().use { scenario ->
248+
val loadingText = "Some text displayed while loading"
249+
250+
// Test that display works
251+
scenario.onActivity { sut ->
252+
sut.showLoadingDialog(loadingText)
253+
}
254+
onView(withText(loadingText))
255+
.check(matches(isDisplayed()))
256+
257+
// Test that hiding works
258+
scenario.onActivity { sut ->
259+
sut.dismissLoadingDialog()
260+
}
261+
onView(allOf(withText(loadingText), isDisplayed()))
262+
.check(doesNotExist())
263+
264+
// Test that there is no timing issue when hiding the dialog directly after.
265+
// This timing issue was reproducible when testing RemoveFilesDialogFragment#removeFiles
266+
// as well as sporadically "in the wild".
267+
scenario.onActivity { sut ->
268+
sut.showLoadingDialog(loadingText)
269+
sut.dismissLoadingDialog()
270+
}
271+
onView(allOf(withText(loadingText), isDisplayed()))
272+
.check(doesNotExist())
273+
// Wait for a potential timing issue - dialog appearing belatedly
274+
Thread.sleep(1000)
275+
onView(allOf(withText(loadingText), isDisplayed()))
276+
.check(doesNotExist())
277+
278+
// Test that multiple display calls after another don't cause a timing issue
279+
scenario.onActivity { sut ->
280+
sut.showLoadingDialog(loadingText)
281+
sut.showLoadingDialog(loadingText)
282+
sut.showLoadingDialog(loadingText)
283+
sut.dismissLoadingDialog()
284+
}
285+
onView(allOf(withText(loadingText), isDisplayed()))
286+
.check(doesNotExist())
287+
// Wait for a potential timing issue - dialog appearing belatedly
288+
Thread.sleep(1000)
289+
onView(allOf(withText(loadingText), isDisplayed()))
290+
.check(doesNotExist())
291+
}
292+
}
239293
}

app/src/androidTest/java/com/nextcloud/test/GrantStoragePermissionRule.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import org.junit.rules.TestRule
1515
import org.junit.runner.Description
1616
import org.junit.runners.model.Statement
1717

18+
/**
19+
* Rule to automatically enable the test to write to the external storage.
20+
* Depending on the SDK version, different approaches might be necessary to achieve the full access.
21+
*/
1822
class GrantStoragePermissionRule private constructor() {
1923

2024
companion object {
@@ -30,6 +34,7 @@ class GrantStoragePermissionRule private constructor() {
3034
private class GrantManageExternalStoragePermissionRule : TestRule {
3135
override fun apply(base: Statement, description: Description): Statement = object : Statement() {
3236
override fun evaluate() {
37+
// Refer to https://developer.android.com/training/data-storage/manage-all-files#enable-manage-external-storage-for-testing
3338
InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand(
3439
"appops set --uid ${InstrumentationRegistry.getInstrumentation().targetContext.packageName} " +
3540
"MANAGE_EXTERNAL_STORAGE allow"

app/src/androidTest/java/com/owncloud/android/AbstractIT.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
package com.owncloud.android;
88

9+
import android.Manifest;
910
import android.accounts.Account;
1011
import android.accounts.AccountManager;
1112
import android.accounts.AuthenticatorException;
@@ -78,6 +79,7 @@
7879
import androidx.test.espresso.contrib.DrawerActions;
7980
import androidx.test.espresso.intent.rule.IntentsTestRule;
8081
import androidx.test.platform.app.InstrumentationRegistry;
82+
import androidx.test.rule.GrantPermissionRule;
8183
import androidx.test.runner.lifecycle.ActivityLifecycleMonitorRegistry;
8284
import androidx.test.runner.lifecycle.Stage;
8385

@@ -93,7 +95,10 @@
9395
*/
9496
public abstract class AbstractIT {
9597
@Rule
96-
public final TestRule permissionRule = GrantStoragePermissionRule.grant();
98+
public final TestRule storagePermissionRule = GrantStoragePermissionRule.grant();
99+
100+
@Rule
101+
public GrantPermissionRule notificationsPermissionRule = GrantPermissionRule.grant(Manifest.permission.POST_NOTIFICATIONS);
97102

98103
protected static OwnCloudClient client;
99104
protected static NextcloudClient nextcloudClient;

app/src/main/java/com/owncloud/android/ui/activity/FileActivity.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,7 @@ protected void updateFileFromDB(){
563563
public void showLoadingDialog(String message) {
564564
runOnUiThread(() -> {
565565
FragmentManager fragmentManager = getSupportFragmentManager();
566+
fragmentManager.executePendingTransactions();
566567
Fragment existingDialog = fragmentManager.findFragmentByTag(DIALOG_WAIT_TAG);
567568

568569
if (existingDialog instanceof LoadingDialog loadingDialog) {
@@ -585,6 +586,7 @@ public void showLoadingDialog(String message) {
585586
public void dismissLoadingDialog() {
586587
runOnUiThread(() -> {
587588
FragmentManager fragmentManager = getSupportFragmentManager();
589+
fragmentManager.executePendingTransactions();
588590
Fragment fragment = fragmentManager.findFragmentByTag(DIALOG_WAIT_TAG);
589591

590592
if (fragment instanceof LoadingDialog loadingDialogFragment) {

0 commit comments

Comments
 (0)