Skip to content

Fix: Instrumented tests are executed in Drone CI again #16612

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.

Sign up for GitHub

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

Open
PhilLab wants to merge 5 commits into
base: master
Choose a base branch
from
+49 −12
Open

Fix: Instrumented tests are executed in Drone CI again #16612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
29e3642
fix(test): Drone CI now runs the instrumented tests again.
PhilLab Feb 28, 2026
c51ee10
fix(test): Upload of instrumented test results now uses correct folder
PhilLab Feb 28, 2026
848cdd5
fix(test): Repaired broken instrumented tests
PhilLab Feb 28, 2026
f0ee559
fix(test): Futile attempt at fixing the UploadDateTests crash
PhilLab Mar 1, 2026
c468198
fix(test) AutoRenameTests don't fail on stable server anymore
PhilLab Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ android {

debug {
enableUnitTestCoverage = project.hasProperty("coverage")
enableAndroidTestCoverage = project.hasProperty("coverage")
Copy link
Contributor Author

@PhilLab PhilLab Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobiasKaminsky with this, Drone now actually runs the instrumented tests again. See https://drone.nextcloud.com/nextcloud/android/28585/1/3 as a proof of failure, i.e. broken tests on master which were never detected.

I'll go ahead addressing the test failures. But there is several other things I noticed:

  1. We should have Drone only run the instrumentation tests. Currently, drone CI runs everything for which coverage is enabled. Why this is chosen as filter, I don't know. But what this means is that Drone wastes time on the unit tests which we already run elsewhere.
    • Unit tests were the only tests Drone ran since Chore: Improve Gradle Configuration #15859. Another weird side-effect of that PR.
    • Easiest for saving drone time would be to disable the coverage for unit tests in the line above, but I guess this is not desired?
  2. Actually, what about the coverage reporting? It doesn't appear anymore. It looks broken, in several ways: - I am not seeing the coverage report on any PR I have recently checked. Don't know when this stopped. Many of the pulls on Codecov just show errors: https://app.codecov.io/github/nextcloud/android/pulls
  3. Coverage reporting should also be extended by the instrumentation tests. I don't see them uploading anything

resConfigs("xxxhdpi")
}
}
Expand Down
5 changes: 2 additions & 3 deletions app/src/androidTest/java/com/nextcloud/ui/SetOnlineStatusBottomSheetIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ class SetOnlineStatusBottomSheetIT : AbstractIT() {
launchActivity<FileDisplayActivity>().use { scenario ->
onView(isRoot()).check(matches(isDisplayed()))

onView(withId(R.id.clearStatusAfterSpinner))
.check(matches(isDisplayed()))

scenario.onActivity { activity ->
val sut = SetOnlineStatusBottomSheet(
Status(StatusType.DND, "Working hard…", "🤖", -1)
)
sut.show(activity.supportFragmentManager, "")
}
onView(withId(R.id.onlineStatus))
.check(matches(isDisplayed()))
}
}
}
7 changes: 6 additions & 1 deletion app/src/androidTest/java/com/nextcloud/ui/SetStatusMessageBottomSheetIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.isRoot
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.rule.GrantPermissionRule
import com.owncloud.android.AbstractIT
import com.owncloud.android.R
import com.owncloud.android.lib.resources.users.ClearAt
import com.owncloud.android.lib.resources.users.PredefinedStatus
import com.owncloud.android.lib.resources.users.Status
Expand All @@ -41,7 +43,6 @@ class SetStatusMessageBottomSheetIT : AbstractIT() {
user,
Status(StatusType.DND, "Working hard…", "🤖", -1)
)
sut.show(activity.supportFragmentManager, "")
val predefinedStatus: ArrayList<PredefinedStatus> = arrayListOf(
PredefinedStatus("meeting", "📅", "In a meeting", ClearAt("period", "3600")),
PredefinedStatus("commuting", "🚌", "Commuting", ClearAt("period", "1800")),
Expand All @@ -51,7 +52,11 @@ class SetStatusMessageBottomSheetIT : AbstractIT() {
PredefinedStatus("vacationing", "🌴", "Vacationing", null)
)
sut.setPredefinedStatus(predefinedStatus)
sut.show(activity.supportFragmentManager, "")
}

onView(withId(R.id.predefinedStatusList))
.check(matches(isDisplayed()))
}
}
}
26 changes: 24 additions & 2 deletions app/src/androidTest/java/com/nextcloud/utils/AutoRenameTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
* Nextcloud - Android Client
*
* SPDX-FileCopyrightText: 2026 Philipp Hasper <vcs@hasper.info>
* SPDX-FileCopyrightText: 2024 Alper Ozturk <alper.ozturk@nextcloud.com>
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
Expand Down Expand Up @@ -243,11 +244,32 @@ class AutoRenameTests : AbstractOnServerIT() {

@Test
fun skipAutoRenameWhenWCFDisabled() {
var capability = capability
val filename = " readme.txt "

// Nextcloud 32+: Respects the isWCFEnabled flag
capability = capability.apply {
versionMayor = NextcloudVersion.nextcloud_32.majorVersionNumber
isWCFEnabled = CapabilityBooleanType.FALSE
}
val filename = " readme.txt "
val result = AutoRename.rename(filename, capability, isFolderPath = true)
var result = AutoRename.rename(filename, capability, isFolderPath = true)
assert(result == filename) { "Expected $filename but got $result" }

// Nextcloud 30-31+: Always applies WCF restrictions, even if flag is set to false
capability = capability.apply {
versionMayor = NextcloudVersion.nextcloud_30.majorVersionNumber
isWCFEnabled = CapabilityBooleanType.FALSE
}
result = AutoRename.rename(filename, capability, isFolderPath = true)
val expectedWCFFilename = "readme.txt"
assert(result == expectedWCFFilename) { "Expected $expectedWCFFilename but got $result" }

// Nextcloud <30: No WCF restrictions, even if flag is set to true
capability = capability.apply {
versionMayor = NextcloudVersion.nextcloud_29.majorVersionNumber
isWCFEnabled = CapabilityBooleanType.TRUE
}
result = AutoRename.rename(filename, capability, isFolderPath = true)
assert(result == filename) { "Expected $filename but got $result" }
}
}
9 changes: 8 additions & 1 deletion app/src/androidTest/java/com/nextcloud/utils/UploadDateTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import com.owncloud.android.utils.DisplayUtils
import io.mockk.MockKAnnotations
import io.mockk.every
import io.mockk.mockkStatic
import io.mockk.unmockkStatic
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertTrue
Expand Down Expand Up @@ -47,10 +49,15 @@ class UploadDateTests {
fun setup() {
context = InstrumentationRegistry.getInstrumentation().context
MockKAnnotations.init(this, relaxed = true)
mockkStatic(System::class)
mockkStatic(System::currentTimeMillis)
Copy link
Contributor Author

@PhilLab PhilLab Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alperozturk96 see the commit message for more details:

fix(test): Futile attempt at fixing the UploadDateTests crash

The tests themselves run through, but immediately after the android process
crashes. Bluntly mocking the whole System class causes problems everywhere.
Even limiting the mocking to a single method still causes the crashes.
The issue is that mocking System.currentTimeMillis() on Android
(ART runtime) is fundamentally unsafe, even with a function reference.
The mock interferes with framework internals (WorkManager, schedulers,
etc.) that run after the test completes, causing the process crash.

You should also see this test crash on your local machine. The test itself is green, but the whole test run is failed:

Image

And the same now happens on the Drone job, now that it correctly picks up all expected tests.

In the best case, the UploadDateTests should be a unit test, instead of an instrumented test. There, I expect the mocking of currentTimeMillis() to not interfere with outside processes. But in the git history I saw that you initially even had that test as unit test and then moved it over. As it is not explained in the commits, I don't know why.
Does it have to do with the context being required for the R.string and for DateUtils.getRelativeDateTimeString?
We could mock it using Robolectric, but that would be a new dependency I don't want to introduce into the repo without consultation.

every { System.currentTimeMillis() } returns JANUARY_27_2026
}

@After
fun tearDown() {
unmockkStatic(System::currentTimeMillis)
}

@Test
fun uploadEntityConvertsToOCUploadAndBackCorrectly() {
val entity = UploadEntity(
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/com/nextcloud/ui/SetStatusMessageBottomSheet.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,10 @@ class SetStatusMessageBottomSheet(val user: User, val currentStatus: Status?) :
@SuppressLint("NotifyDataSetChanged")
@VisibleForTesting
fun setPredefinedStatus(predefinedStatus: ArrayList<PredefinedStatus>) {
adapter.list = predefinedStatus
binding.predefinedStatusList.adapter?.notifyDataSetChanged()
this.predefinedStatus = predefinedStatus
if (this::adapter.isInitialized) {
adapter.list = predefinedStatus
binding.predefinedStatusList.adapter?.notifyDataSetChanged()
}
}
}
6 changes: 3 additions & 3 deletions scripts/uploadReport.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ if [ -z $USER ] || [ -z $PASS ]; then
fi

if [ $TYPE = "IT" ]; then
FOLDER=app/build/reports/androidTests/connected/flavors/gplay
FOLDER=app/build/reports/androidTests/connected/debug/flavors/gplay
Copy link
Contributor Author

@PhilLab PhilLab Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobiasKaminsky these adjustments are needed so the report files are actually found. But reporting still is broken, which you can see e.g. in the log of https://drone.nextcloud.com/nextcloud/android/28585/1/3:

It does upload the report to https://www.kaminsky.me/nc-dev/android-integrationTests/28585-IT-stable-10-44 but it fails to create the expected Github comment notifying the developer of this.
The error is:

{
  "message": "Must have admin rights to Repository.",
  "documentation_url": "https://docs.github.com/rest/issues/comments#create-an-issue-comment",
  "status": "403"
}

elif [ $TYPE = "Unit" ]; then
FOLDER=app/build/reports/tests/testGplayDebugUnitTest
else
Expand All @@ -68,10 +68,10 @@ else
-X POST https://api.github.com/repos/nextcloud/android/issues/$PR/comments \
-d "{ \"body\" : \"$BRANCH_TYPE test failed, but no output was generated. Maybe a preliminary stage failed. \" }"

if [ -e app/build/reports/androidTests/connected/flavors/gplay ] ; then
if [ -e app/build/reports/androidTests/connected/debug/flavors/gplay ] ; then
TYPE="IT"
BRANCH_TYPE=$BRANCH-$TYPE
upload "app/build/reports/androidTests/connected/flavors/gplay"
upload "app/build/reports/androidTests/connected/debug/flavors/gplay"
fi

if [ -e app/build/reports/tests/testGplayDebugUnitTest ] ; then
Expand Down
Loading