diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5fab39edd484..510fd5e94885 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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) @@ -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 @@ -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 @@ -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 diff --git a/README.md b/README.md index bd99dd07fe89..1a23a124eb06 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/app/src/androidTest/java/com/nextcloud/test/GrantStoragePermissionRule.kt b/app/src/androidTest/java/com/nextcloud/test/GrantStoragePermissionRule.kt index b310a897fa42..3bade62247c4 100644 --- a/app/src/androidTest/java/com/nextcloud/test/GrantStoragePermissionRule.kt +++ b/app/src/androidTest/java/com/nextcloud/test/GrantStoragePermissionRule.kt @@ -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 { @@ -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" diff --git a/app/src/androidTest/java/com/owncloud/android/AbstractIT.java b/app/src/androidTest/java/com/owncloud/android/AbstractIT.java index 5ea27541062f..0c213f846757 100644 --- a/app/src/androidTest/java/com/owncloud/android/AbstractIT.java +++ b/app/src/androidTest/java/com/owncloud/android/AbstractIT.java @@ -6,6 +6,7 @@ */ package com.owncloud.android; +import android.Manifest; import android.accounts.Account; import android.accounts.AccountManager; import android.accounts.AuthenticatorException; @@ -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; @@ -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; @@ -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); } } diff --git a/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java b/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java index aef7eee1110c..2b1428ed551a 100644 --- a/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java +++ b/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java @@ -36,6 +36,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; @@ -54,7 +55,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 @@ -121,6 +131,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()); @@ -138,6 +153,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); } diff --git a/app/src/androidTest/java/com/owncloud/android/DownloadIT.java b/app/src/androidTest/java/com/owncloud/android/DownloadIT.java index 610469f4d3fa..f8fd99517f1c 100644 --- a/app/src/androidTest/java/com/owncloud/android/DownloadIT.java +++ b/app/src/androidTest/java/com/owncloud/android/DownloadIT.java @@ -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/"; @@ -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/" + Uri.encode(account.name, "@") + "/testUpload/nonEmpty2.txt", file2.getStoragePath()); }