-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Verify test failure #16329
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
base: master
Are you sure you want to change the base?
Verify test failure #16329
Changes from all commits
c3d9706
be8bfbb
95d131e
d156ccb
71ac686
0becd05
dbd4897
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /* | ||
| * Nextcloud - Android Client | ||
| * | ||
| * SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info> | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| package com.nextcloud.test | ||
|
|
||
| import com.nextcloud.client.network.Connectivity | ||
| import com.nextcloud.client.network.ConnectivityService | ||
|
|
||
| /** A mocked connectivity service returning that the device is offline **/ | ||
| class ConnectivityServiceOfflineMock : ConnectivityService { | ||
| override fun isNetworkAndServerAvailable(callback: ConnectivityService.GenericCallback<Boolean>) { | ||
| callback.onComplete(false) | ||
| } | ||
|
|
||
| override fun isConnected(): Boolean = false | ||
|
|
||
| override fun isInternetWalled(): Boolean = false | ||
|
|
||
| override fun getConnectivity(): Connectivity = Connectivity.CONNECTED_WIFI | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| /* | ||
| * Nextcloud - Android Client | ||
| * | ||
| * SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info> | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| package com.nextcloud.test | ||
|
|
||
| import androidx.test.espresso.IdlingResource | ||
| import com.owncloud.android.datamodel.FileDataStorageManager | ||
| import com.owncloud.android.datamodel.OCFile | ||
| import java.util.concurrent.atomic.AtomicLong | ||
| import java.util.concurrent.atomic.AtomicReference | ||
|
|
||
| /** | ||
| * IdlingResource that can be reused to watch the removal of different file ids sequentially. | ||
| * | ||
| * Use setFileId(fileId) before triggering the deletion. The resource will call the Espresso callback | ||
| * once the file no longer exists. Call unregister from IdlingRegistry in @After. | ||
| */ | ||
| class FileRemovedIdlingResource(private val storageManager: FileDataStorageManager) : IdlingResource { | ||
| private var resourceCallback: IdlingResource.ResourceCallback? = null | ||
|
|
||
| // null means "no file set" | ||
| private var currentFile = AtomicReference<OCFile>(null) | ||
|
|
||
| override fun getName(): String = "${this::class.java.simpleName}" | ||
|
|
||
| override fun isIdleNow(): Boolean { | ||
| val file = currentFile.get() | ||
| // If no file set, consider idle. If file set, idle only if it doesn't exist. | ||
| val idle = file == null || (!storageManager.fileExists(file.fileId) && !file.exists()) | ||
| if (idle && file != null) { | ||
| // if we detect it's already removed, notify and clear | ||
| resourceCallback?.onTransitionToIdle() | ||
| currentFile.set(null) | ||
| } | ||
| return idle | ||
| } | ||
|
|
||
| override fun registerIdleTransitionCallback(callback: IdlingResource.ResourceCallback) { | ||
| this.resourceCallback = callback | ||
| } | ||
|
|
||
| /** | ||
| * Start watching the given file. Call this right before performing the UI action that triggers deletion. | ||
| */ | ||
| fun setFile(file: OCFile) { | ||
| currentFile.set(file) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /* | ||
| * Nextcloud - Android Client | ||
| * | ||
| * SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info> | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| package com.nextcloud.test | ||
|
|
||
| import android.content.Context | ||
| import android.view.View | ||
| import androidx.test.espresso.FailureHandler | ||
| import androidx.test.espresso.base.DefaultFailureHandler | ||
| import org.hamcrest.Matcher | ||
|
|
||
| /** | ||
| * When testing inside of a loop, test failures are hard to attribute. For that, wrap them in an outer | ||
| * exception detailing more about the context. | ||
| * | ||
| * Set the failure handler via | ||
| * ``` | ||
| * Espresso.setFailureHandler( | ||
| * LoopFailureHandler(targetContext, "Test failed in iteration $yourTestIterationCounter") | ||
| * ) | ||
| * ``` | ||
| * and set it back to the default afterwards via | ||
| * ``` | ||
| * Espresso.setFailureHandler(DefaultFailureHandler(targetContext)) | ||
| * ``` | ||
| */ | ||
| class LoopFailureHandler(targetContext: Context, private val loopMessage: String) : FailureHandler { | ||
| private val delegate: FailureHandler = DefaultFailureHandler(targetContext) | ||
|
|
||
| override fun handle(error: Throwable?, viewMatcher: Matcher<View?>?) { | ||
| // Wrap in additional Exception | ||
| delegate.handle(Exception(loopMessage, error), viewMatcher) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ import com.owncloud.android.utils.ScreenshotTest | |
| import org.junit.After | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertNotNull | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Before | ||
| import org.junit.Test | ||
| import java.util.Random | ||
|
|
@@ -120,6 +121,8 @@ class GalleryFragmentIT : AbstractIT() { | |
| onView(withId(R.id.list_root)) | ||
| .perform(RecyclerViewActions.scrollToLastPosition<GalleryRowHolder>()) | ||
| .perform(RecyclerViewActions.scrollToPosition<GalleryRowHolder>(0)) | ||
|
|
||
| assertTrue("Do not merge - just testing test failures", false) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the third failure I provoke
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
|
|
||
| // Test selection of all entries | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,230 @@ | ||
| /* | ||
| * Nextcloud - Android Client | ||
| * | ||
| * SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info> | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
| package com.owncloud.android.ui.preview | ||
|
|
||
| import androidx.appcompat.widget.ActionBarContainer | ||
| import androidx.test.core.app.launchActivity | ||
| import androidx.test.espresso.Espresso | ||
| import androidx.test.espresso.Espresso.onView | ||
| import androidx.test.espresso.IdlingRegistry | ||
| import androidx.test.espresso.action.ViewActions | ||
| import androidx.test.espresso.assertion.ViewAssertions.matches | ||
| import androidx.test.espresso.base.DefaultFailureHandler | ||
| import androidx.test.espresso.matcher.RootMatchers.isDialog | ||
| import androidx.test.espresso.matcher.ViewMatchers.isAssignableFrom | ||
| import androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA | ||
| import androidx.test.espresso.matcher.ViewMatchers.isDisplayed | ||
| import androidx.test.espresso.matcher.ViewMatchers.isRoot | ||
| import androidx.test.espresso.matcher.ViewMatchers.withId | ||
| import androidx.test.espresso.matcher.ViewMatchers.withText | ||
| import com.nextcloud.test.ConnectivityServiceOfflineMock | ||
| import com.nextcloud.test.FileRemovedIdlingResource | ||
| import com.nextcloud.test.LoopFailureHandler | ||
| import com.owncloud.android.AbstractOnServerIT | ||
| import com.owncloud.android.R | ||
| import com.owncloud.android.datamodel.OCFile | ||
| import com.owncloud.android.db.OCUpload | ||
| import com.owncloud.android.files.services.NameCollisionPolicy | ||
| import com.owncloud.android.lib.resources.files.ExistenceCheckRemoteOperation | ||
| import org.hamcrest.Matchers.allOf | ||
| import org.junit.After | ||
| import org.junit.Assert.assertFalse | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Before | ||
| import org.junit.Ignore | ||
| import org.junit.Test | ||
| import java.io.File | ||
|
|
||
| class PreviewImageActivityIT : AbstractOnServerIT() { | ||
| companion object { | ||
| private const val REMOTE_FOLDER: String = "/PreviewImageActivityIT/" | ||
| } | ||
|
|
||
| var fileRemovedIdlingResource = FileRemovedIdlingResource(storageManager) | ||
|
|
||
| @Suppress("SameParameterValue") | ||
| private fun createLocalMockedImageFiles(count: Int): List<OCFile> { | ||
| val srcPngFile = getFile("imageFile.png") | ||
| return (0 until count).map { i -> | ||
| val pngFile = File(srcPngFile.parent ?: ".", "image$i.png") | ||
| srcPngFile.copyTo(pngFile, overwrite = true) | ||
|
|
||
| OCFile("/${pngFile.name}").apply { | ||
| storagePath = pngFile.absolutePath | ||
| mimeType = "image/png" | ||
| modificationTimestamp = 1000000 | ||
| permissions = "D" // OCFile.PERMISSION_CAN_DELETE_OR_LEAVE_SHARE. Required for deletion button to show | ||
| }.also { | ||
| storageManager.saveNewFile(it) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Create image files and upload them to the connected server. | ||
| * | ||
| * This function relies on the images not existing beforehand, as AbstractOnServerIT#deleteAllFilesOnServer() | ||
| * should clean up. If it does fail, likely because that clean up didn't work and there are leftovers from | ||
| * a previous run | ||
| * @param count Number of files to create | ||
| * @param folder Parent folder to which to upload. Must start and end with a slash | ||
| */ | ||
| private fun createAndUploadImageFiles(count: Int, folder: String = REMOTE_FOLDER): List<OCFile> { | ||
| val srcPngFile = getFile("imageFile.png") | ||
| return (0 until count).map { i -> | ||
| val pngFile = File(srcPngFile.parent ?: ".", "image$i.png") | ||
| srcPngFile.copyTo(pngFile, overwrite = true) | ||
|
|
||
| val ocUpload = OCUpload( | ||
| pngFile.absolutePath, | ||
| folder + pngFile.name, | ||
| account.name | ||
| ).apply { | ||
| nameCollisionPolicy = NameCollisionPolicy.OVERWRITE | ||
| } | ||
| uploadOCUpload(ocUpload) | ||
|
|
||
| fileDataStorageManager.getFileByDecryptedRemotePath(folder + pngFile.name)!! | ||
| } | ||
| } | ||
|
|
||
| private fun veryImageThenDelete(testFile: OCFile) { | ||
| Espresso.setFailureHandler( | ||
| LoopFailureHandler(targetContext, "Test failed with image file ${testFile.fileName}") | ||
| ) | ||
|
|
||
| assertTrue(testFile.exists()) | ||
| assertTrue(testFile.fileExists()) | ||
|
|
||
| onView(withId(R.id.image)) | ||
| .check(matches(isDisplayed())) | ||
|
|
||
| // Check that the Action Bar shows the file name as title | ||
| onView( | ||
| allOf( | ||
| isDescendantOfA(isAssignableFrom(ActionBarContainer::class.java)), | ||
| withText(testFile.fileName) | ||
| ) | ||
| ).check(matches(isDisplayed())) | ||
|
|
||
| // Open the Action Bar's overflow menu. | ||
| // The official way would be: | ||
| // openActionBarOverflowOrOptionsMenu(targetContext) | ||
| // But this doesn't find the view. Presumably because Espresso.OVERFLOW_BUTTON_MATCHER looks for the description | ||
| // "More options", whereas it actually says "More menu". | ||
| // selecting by this would also work: | ||
| // onView(withContentDescription("More menu")).perform(ViewActions.click()) | ||
| // For now, we identify it by the ID we know it to be | ||
| onView(withId(R.id.custom_menu_placeholder_item)).perform(ViewActions.click()) | ||
|
|
||
| // Click the "Remove" button | ||
| onView(withText(R.string.common_remove)).perform(ViewActions.click()) | ||
|
|
||
| // Check confirmation dialog and then confirm the deletion by clicking the main button of the dialog | ||
| val expectedText = targetContext.getString(R.string.confirmation_remove_file_alert, testFile.fileName) | ||
| onView(withId(android.R.id.message)) | ||
| .inRoot(isDialog()) | ||
| .check(matches(withText(expectedText))) | ||
|
|
||
| onView(withId(android.R.id.button1)) | ||
| .inRoot(isDialog()) | ||
| .check(matches(withText(R.string.file_delete))) | ||
| .perform(ViewActions.click()) | ||
|
|
||
| // Register the idling resource to wait for successful deletion | ||
| fileRemovedIdlingResource.setFile(testFile) | ||
|
|
||
| // Wait for idle, then verify that the file is gone. Somehow waitForIdleSync() doesn't work and we need onIdle() | ||
| Espresso.onIdle() | ||
| assertFalse("test file still exists: ${testFile.fileName}", testFile.exists()) | ||
|
|
||
| Espresso.setFailureHandler(DefaultFailureHandler(targetContext)) | ||
| } | ||
|
|
||
| private fun executeDeletionTestScenario( | ||
| localOnly: Boolean, | ||
| offline: Boolean, | ||
| fileListTransformation: (List<OCFile>) -> List<OCFile> | ||
| ) { | ||
| val imageCount = 5 | ||
| val testFiles = if (localOnly) { | ||
| createLocalMockedImageFiles( | ||
| imageCount | ||
| ) | ||
| } else { | ||
| createAndUploadImageFiles(imageCount) | ||
| } | ||
| val expectedFileOrder = fileListTransformation(testFiles) | ||
|
|
||
| val intent = PreviewImageActivity.previewFileIntent(targetContext, user, expectedFileOrder.first()) | ||
| launchActivity<PreviewImageActivity>(intent).use { scenario -> | ||
| if (offline) { | ||
| scenario.onActivity { activity -> | ||
| activity.connectivityService = ConnectivityServiceOfflineMock() | ||
| } | ||
| } | ||
| onView(isRoot()).check(matches(isDisplayed())) | ||
|
|
||
| assertTrue("Do not merge - just testing test failures", false) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is where I provoke the failure
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was not catched - https://drone.nextcloud.com/nextcloud/android/27697 ran through. And no other test job failed, either. For reference, this is the overview of GitHub's checks for this commit, but somehow it does not show the drone action... Very confusing... https://github.com/nextcloud/android/pull/16329/checks?sha=71ac686c47076f8d24c71eae1bdcadf06214eeba But as this view confusingly doesn't list the Drone job, I'll leave this screenshot as well, at the state of this commit. Drone job is shown green: |
||
|
|
||
| for (testFile in expectedFileOrder) { | ||
| veryImageThenDelete(testFile) | ||
| assertTrue( | ||
| "Test file still exists on the server: ${testFile.remotePath}", | ||
| ExistenceCheckRemoteOperation(testFile.remotePath, true).execute(client).isSuccess | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun testDeleteFromSlideshow_impl(localOnly: Boolean, offline: Boolean) { | ||
| // Case 1: start at first image | ||
| executeDeletionTestScenario(localOnly, offline) { list -> list } | ||
| // Case 2: start at last image (reversed) | ||
| executeDeletionTestScenario(localOnly, offline) { list -> list.reversed() } | ||
| // Case 3: Start in the middle. From middle to the end, then backwards through remaining files of the first half | ||
| executeDeletionTestScenario(localOnly, offline) { list -> | ||
| list.subList(list.size / 2, list.size) + list.subList(0, list.size / 2).reversed() | ||
| } | ||
| } | ||
|
|
||
| @Before | ||
| fun bringUp() { | ||
| IdlingRegistry.getInstance().register(fileRemovedIdlingResource) | ||
| } | ||
|
|
||
| @After | ||
| fun tearDown() { | ||
| IdlingRegistry.getInstance().unregister(fileRemovedIdlingResource) | ||
| } | ||
|
|
||
| @Test | ||
| fun deleteFromSlideshow_localOnly_online() { | ||
| testDeleteFromSlideshow_impl(localOnly = true, offline = false) | ||
| } | ||
|
|
||
| @Test | ||
| fun deleteFromSlideshow_localOnly_offline() { | ||
| testDeleteFromSlideshow_impl(localOnly = true, offline = true) | ||
| } | ||
|
|
||
| @Test | ||
| fun deleteFromSlideshow_remote_online() { | ||
| testDeleteFromSlideshow_impl(localOnly = false, offline = false) | ||
| } | ||
|
|
||
| @Test | ||
| @Ignore( | ||
| "Offline deletion is following a different UX and it is also brittle: Deletion might happen 10 minutes later" | ||
| ) | ||
| fun deleteFromSlideshow_remote_offline() { | ||
| // Note: the offline mock doesn't actually do what it is supposed to. The OfflineOperationsWorker uses its | ||
| // own connectivityService, which is online, and may still execute the server deletion. | ||
| // You'll need to address this, should you activate that test. Otherwise it might not catch all error cases | ||
| testDeleteFromSlideshow_impl(localOnly = false, offline = true) | ||
| } | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the second failure I provoke
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also wasn't catched - https://drone.nextcloud.com/nextcloud/android/27788 ran through and no other test job ran the test either.
For reference, this is the overview of checks for this commit: https://github.com/nextcloud/android/pull/16329/checks?sha=0becd054d1b3eefad3327626ea3d5b79cf397eba.
And the screenshot
