Skip to content

Minor improvements for automated testing #15424

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
wants to merge 5 commits into
base: master
Choose a base branch
from
+73 −14
Open

Minor improvements for automated testing #15424

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
c287696
Minor improvements of testing documentation
PhilLab Aug 24, 2025
4c07da7
Instrumentation tests now automatically grab the notifications permis…
PhilLab Aug 24, 2025
f6a235c
DownloadIT now works for all build variants
PhilLab Oct 28, 2025
db778af
Fixed AbstractIT deleting the wrong account
PhilLab Nov 2, 2025
ba0751b
AbstractOnServerIT file deletion now ignores non-empty encrypted folders
PhilLab Nov 2, 2025
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
42 changes: 34 additions & 8 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
1. [Contributing to Source Code](#contributing-to-source-code)
1. [Developing process](#developing-process)
1. [Branching model](#branching-model)
1. [Android Studio formatter setup](#android-studio-formatter-setup)
1. [Formatter setup](#formatter-setup)
1. [Build variants](#build-variants)
1. [Git hooks](#git-hooks)
1. [Contribution process](#contribution-process)
Expand Down Expand Up @@ -107,12 +107,22 @@ We are all about quality while not sacrificing speed so we use a very pragmatic
* Hot fixes not relevant for an upcoming feature release but the latest release can target the bug fix branch directly


### Android Studio formatter setup
### Formatter setup
Our formatter setup is rather simple:
* Standard Android Studio
* Line length 120 characters (Settings->Editor->Code Style->Right margin(columns): 120)
* Line length 120 characters (Settings->Editor->Code Style->Right margin(columns): 120; also set by EditorConfig
* Auto optimize imports (Settings->Editor->Auto Import->Optimize imports on the fly)

You can fix Check / check (spotlessKotlinCheck) via following commands:

```bash
./gradlew spotlessApply
./gradlew detekt
./gradlew spotlessCheck
./gradlew spotlessKotlinCheck
```

See section [Git hooks](#git-hooks) to have these run automatically with your commits.

### Build variants
There are three build variants
Expand All @@ -122,7 +132,7 @@ There are three build variants

### Git hooks
We provide git hooks to make development process easier for both the developer and the reviewers.
To install them, just run:
They are stored in [/scripts/hooks](/scripts/hooks) and can be installed with:

```bash
./gradlew installGitHooks
Expand Down Expand Up @@ -214,21 +224,37 @@ Source code of app:
- small, isolated tests, with no need of Android SDK
- code coverage can be directly shown via right click on test and select "Run Test with Coverage"

```
./gradlew jacocoTestGplayDebugUnitTest
```bash

#### Instrumented tests
- tests to see larger code working in correct way
- tests that require parts of Android SDK
- best to avoid server communication, see https://github.com/nextcloud/android/pull/3624

- run all tests ```./gradlew createGplayDebugCoverageReport -Pcoverage=true```
- run selective test class: ```./gradlew createGplayDebugCoverageReport -Pcoverage=true
-Pandroid.testInstrumentationRunnerArguments.class=com.owncloud.android.datamodel.FileDataStorageManagerTest```
-Pandroid.testInstrumentationRunnerArguments.class=com.owncloud.android.datamodel.FileDataStorageManagerContentProviderClientIT```
- run multiple test classes:
- separate by ","
- ```./gradlew createGplayDebugCoverageReport -Pcoverage=true -Pandroid.testInstrumentationRunnerArguments.class=com.owncloud.android.datamodel.FileDataStorageManagerTest,com.nextcloud.client.FileDisplayActivityIT```
- ```./gradlew createGplayDebugCoverageReport -Pcoverage=true -Pandroid.testInstrumentationRunnerArguments.class=com.owncloud.android.datamodel.FileDataStorageManagerContentProviderClientIT,com.nextcloud.client.FileDisplayActivityIT```
- run one test in class: ```./gradlew createGplayDebugCoverageReport -Pcoverage=true
-Pandroid.testInstrumentationRunnerArguments.class=com.owncloud.android.datamodel.FileDataStorageManagerTest#saveNewFile```
-Pandroid.testInstrumentationRunnerArguments.class=com.owncloud.android.datamodel.FileDataStorageManagerContentProviderClientIT#saveFile```
- JaCoCo results are shown as html: firefox ./build/reports/coverage/gplay/debug/index.html

#### Instrumented tests with server communication
It is best to avoid server communication, see https://github.com/nextcloud/android/pull/3624.
But if a test requires a server, this is how it is done:
- Have a Nextcloud service reachable by your test device. This can be an existing server in the internet or a locally deployed one
as per the [server developer documentation](https://docs.nextcloud.com/server/latest/developer_manual/getting_started/devenv.html)
- Create a separate(!) test user on that server, otherwise the tests will infer with productive data.
- In `gradle.properties`, enter the URL, user name and password via the `NC_TEST_SERVER_...` attributes.
If you want to prevent an accidental commit of those, you can also store them in `~/.gradle/gradle.properties`.
- Your test class should inherit from `AbstractOnServerIT`, e.g.: `public class DownloadIT extends AbstractOnServerIT { ...`
Note that this will automatically delete all files after each test run, so you absolutely NEED a separate test user as mentioned above.
- All preconditions of your test regarding server data, e.g. existing files, need to be established by your test itself.
As a reference, see how `DownloadIT` first uploads the files it later tests the download with.
- Clean up these preconditions again, also in the failure case, using one or multiple `@After` methods in your test class.

#### UI tests
We use [shot](https://github.com/Karumi/Shot) for taking screenshots and compare them
Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ If you want to [contribute](https://nextcloud.com/contribute/) to the Nextcloud
* reporting problems / suggesting enhancements by [opening new issues](https://github.com/nextcloud/android/issues/new/choose)
* implementing proposed bug fixes and enhancement ideas by submitting PRs (associated with a corresponding issue preferably)
* reviewing [pull requests](https://github.com/nextcloud/android/pulls) and providing feedback on code, implementation, and functionality
* Add [automated tests](CONTRIBUTING.md#testing) for existing functionality
* installing and testing [pull request builds](https://github.com/nextcloud/android/pulls), [daily/dev builds](https://github.com/nextcloud/android#development-version-hammer), or [RCs/release candidate builds](https://github.com/nextcloud/android/releases)
* enhancing Admin, User, or Developer [documentation](https://github.com/nextcloud/documentation/)
* hitting hard on the latest stable release by testing fundamental features and evaluating the user experience
Expand Down
5 changes: 5 additions & 0 deletions app/src/androidTest/java/com/nextcloud/test/GrantStoragePermissionRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import org.junit.rules.TestRule
import org.junit.runner.Description
import org.junit.runners.model.Statement

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

companion object {
Expand All @@ -30,6 +34,7 @@ class GrantStoragePermissionRule private constructor() {
private class GrantManageExternalStoragePermissionRule : TestRule {
override fun apply(base: Statement, description: Description): Statement = object : Statement() {
override fun evaluate() {
// Refer to https://developer.android.com/training/data-storage/manage-all-files#enable-manage-external-storage-for-testing
InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand(
"appops set --uid ${InstrumentationRegistry.getInstrumentation().targetContext.packageName} " +
"MANAGE_EXTERNAL_STORAGE allow"
Expand Down
9 changes: 7 additions & 2 deletions app/src/androidTest/java/com/owncloud/android/AbstractIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package com.owncloud.android;

import android.Manifest;
import android.accounts.Account;
import android.accounts.AccountManager;
import android.accounts.AuthenticatorException;
Expand Down Expand Up @@ -78,6 +79,7 @@
import androidx.test.espresso.contrib.DrawerActions;
import androidx.test.espresso.intent.rule.IntentsTestRule;
import androidx.test.platform.app.InstrumentationRegistry;
import androidx.test.rule.GrantPermissionRule;
import androidx.test.runner.lifecycle.ActivityLifecycleMonitorRegistry;
import androidx.test.runner.lifecycle.Stage;

Expand All @@ -93,7 +95,10 @@
*/
public abstract class AbstractIT {
@Rule
public final TestRule permissionRule = GrantStoragePermissionRule.grant();
public final TestRule storagePermissionRule = GrantStoragePermissionRule.grant();

@Rule
public GrantPermissionRule notificationsPermissionRule = GrantPermissionRule.grant(Manifest.permission.POST_NOTIFICATIONS);

protected static OwnCloudClient client;
protected static NextcloudClient nextcloudClient;
Expand All @@ -118,7 +123,7 @@ public static void beforeAll() {
AccountManager platformAccountManager = AccountManager.get(targetContext);

for (Account account : platformAccountManager.getAccounts()) {
if (account.type.equalsIgnoreCase("nextcloud")) {
if (account.type.equalsIgnoreCase(MainApp.getAccountType(targetContext))) {
platformAccountManager.removeAccountExplicitly(account);
}
}
Expand Down
24 changes: 23 additions & 1 deletion app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.owncloud.android.lib.resources.files.model.RemoteFile;
import com.owncloud.android.operations.RefreshFolderOperation;
import com.owncloud.android.operations.UploadFileOperation;
import com.owncloud.android.utils.MimeType;

import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.httpclient.methods.GetMethod;
Expand All @@ -53,7 +54,16 @@
import static org.junit.Assert.assertTrue;

/**
* Common base for all integration tests.
* Common base for all integration tests requiring a server connection.
* ATTENTION: Deletes ALL files of the test user on the server after each test run.
* So you MUST use a dedicated test user.
*
* Uses server, user and password given as `testInstrumentationRunnerArgument`
* - TEST_SERVER_URL
* - TEST_SERVER_USERNAME
* - TEST_SERVER_PASSWORD
* These are supplied via build.gradle, which takes them from gradle.properties.
* So look in the latter file to set to your own server & test user.
*/
public abstract class AbstractOnServerIT extends AbstractIT {
@BeforeClass
Expand Down Expand Up @@ -120,6 +130,11 @@ public void after() {
super.after();
}

private static boolean isFolder(RemoteFile file) {
// TODO: should probably move to RemoteFile class
return MimeType.DIRECTORY.equals(file.getMimeType()) || MimeType.WEBDAV_FOLDER.equals(file.getMimeType());
}

public static void deleteAllFilesOnServer() {
RemoteOperationResult result = new ReadFolderRemoteOperation("/").execute(client);
assertTrue(result.getLogMessage(), result.isSuccess());
Expand All @@ -137,6 +152,13 @@ public static void deleteAllFilesOnServer() {
.execute(client)
.isSuccess();

if (!operationResult && isFolder(remoteFile)) {
// Deleting encrypted folder is not possible due to bug
// https://github.com/nextcloud/end_to_end_encryption/issues/421
// Toggling encryption also fails, when the folder is not empty. So we ignore this folder
continue;
}

assertTrue(operationResult);
}

Expand Down
6 changes: 3 additions & 3 deletions app/src/androidTest/java/com/owncloud/android/DownloadIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import static org.junit.Assert.assertTrue;

/**
* Tests related to file uploads.
* Tests related to file downloads.
*/
public class DownloadIT extends AbstractOnServerIT {
private static final String FOLDER = "/testUpload/";
Expand Down Expand Up @@ -98,10 +98,10 @@ private void verifyDownload(OCFile file1, OCFile file2) {
assertTrue(new File(file2.getStoragePath()).exists());

// test against hardcoded path to make sure that it is correct
assertEquals("/storage/emulated/0/Android/media/com.nextcloud.client/nextcloud/" +
assertEquals("/storage/emulated/0/Android/media/"+targetContext.getPackageName()+"/nextcloud/" +
Uri.encode(account.name, "@") + "/testUpload/nonEmpty.txt",
file1.getStoragePath());
assertEquals("/storage/emulated/0/Android/media/com.nextcloud.client/nextcloud/" +
assertEquals("/storage/emulated/0/Android/media/"+targetContext.getPackageName()+"/nextcloud/" +
Copy link
Member

@tobiasKaminsky tobiasKaminsky Nov 26, 2025

Choose a reason for hiding this comment

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

Is there a reason to change that?
We test "only" our official Nextcloud client.

Copy link
Contributor Author

@PhilLab PhilLab Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@tobiasKaminsky Two reasons:

  1. The issue is that when you switch between test development and app debugging (to fix the found issues), the tests always deletes the data and when starting the app you need to login again. This costs me 2-3 minutes every time and is quite annoying.
    A good workaround for this is to start the tests under one build variant, and debug the app under another build variant. But for this to work, it needs to be respected everywhere, otherwise assertions will fail or even the wrong data be deleted. This is the common background of the two commits Fixed AbstractIT deleting the wrong account and DownloadIT now works for all build variants
  2. Also conceptually it is clearer that instead of the magic string com.nextcloud.client it refers to what is semantically needed here - and that is the packageName. Sure, following that paradigm, the assertion should make use of even more functions (e.g. to retrieve the /media/ path programmatically instead of hard-coded). But I didn't have an actual use case to justify that. Only the use case from #1.

Uri.encode(account.name, "@") + "/testUpload/nonEmpty2.txt",
file2.getStoragePath());
}
Expand Down