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/build.gradle.kts b/app/build.gradle.kts index 057ec2bdd451..03efc38bca56 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -75,7 +75,7 @@ val ndkEnv = buildMap { } val configProps = Properties().apply { - val file = rootProject.file(".gradle/config.properties") + val file = rootProject.file("gradle.properties") if (file.exists()) load(FileInputStream(file)) } 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..80a19c4f3020 100644 --- a/app/src/androidTest/java/com/owncloud/android/AbstractIT.java +++ b/app/src/androidTest/java/com/owncloud/android/AbstractIT.java @@ -6,12 +6,12 @@ */ package com.owncloud.android; +import android.Manifest; import android.accounts.Account; import android.accounts.AccountManager; import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; import android.app.Activity; -import android.content.ActivityNotFoundException; import android.content.Context; import android.content.res.Configuration; import android.content.res.Resources; @@ -55,7 +55,6 @@ import com.owncloud.android.operations.CreateFolderOperation; import com.owncloud.android.operations.UploadFileOperation; import com.owncloud.android.utils.FileStorageUtils; -import com.owncloud.android.utils.theme.MaterialSchemesProvider; import org.apache.commons.io.FileUtils; import org.junit.After; @@ -78,6 +77,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 +93,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 +121,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); } } @@ -426,7 +429,7 @@ public boolean isPowerSavingEnabled() { newUpload.setRemoteFolderToBeCreated(); - RemoteOperationResult result = newUpload.execute(client); + var result = newUpload.execute(client); assertTrue(result.getLogMessage(), result.isSuccess()); } @@ -531,8 +534,8 @@ protected static Account createAccount(String name) { platformAccountManager.setUserData(temp, KEY_USER_ID, name.substring(0, atPos)); Account account = UserAccountManagerImpl.fromContext(targetContext).getAccountByName(name); - if (account == null) { - throw new ActivityNotFoundException(); + if (Objects.equals(account.type, targetContext.getString(R.string.anonymous_account_type))) { + throw new RuntimeException("Could not get account with name " + name); } return account; } @@ -541,37 +544,7 @@ protected static boolean removeAccount(Account account) { return AccountManager.get(targetContext).removeAccountExplicitly(account); } - protected MaterialSchemesProvider getMaterialSchemesProvider() { - return new MaterialSchemesProvider() { - @NonNull - @Override - public MaterialSchemes getMaterialSchemesForUser(@NonNull User user) { - return null; - } - - @NonNull - @Override - public MaterialSchemes getMaterialSchemesForCapability(@NonNull OCCapability capability) { - return null; - } - - @NonNull - @Override - public MaterialSchemes getMaterialSchemesForCurrentUser() { - return new MaterialSchemesImpl(R.color.primary, false); - } - - @NonNull - @Override - public MaterialSchemes getDefaultMaterialSchemes() { - return null; - } - - @NonNull - @Override - public MaterialSchemes getMaterialSchemesForPrimaryBackground() { - return null; - } - }; + protected MaterialSchemes getMaterialSchemesForCurrentUser() { + return new MaterialSchemesImpl(R.color.primary, false); } } diff --git a/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java b/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java index aef7eee1110c..1b0e1b8d3c17 100644 --- a/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java +++ b/app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java @@ -10,7 +10,6 @@ import android.accounts.AccountManager; import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; -import android.content.ActivityNotFoundException; import android.net.Uri; import android.os.Bundle; @@ -29,13 +28,13 @@ import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.OwnCloudClientFactory; import com.owncloud.android.lib.common.accounts.AccountUtils; -import com.owncloud.android.lib.common.operations.RemoteOperationResult; import com.owncloud.android.lib.resources.e2ee.ToggleEncryptionRemoteOperation; import com.owncloud.android.lib.resources.files.ReadFolderRemoteOperation; import com.owncloud.android.lib.resources.files.RemoveFileRemoteOperation; 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; @@ -45,6 +44,7 @@ import java.io.File; import java.io.IOException; +import java.util.Objects; import java.util.Optional; import androidx.annotation.NonNull; @@ -53,9 +53,15 @@ import static org.junit.Assert.assertNotNull; 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 public static void beforeAll() { @@ -90,8 +96,8 @@ public static void beforeAll() { final UserAccountManager userAccountManager = UserAccountManagerImpl.fromContext(targetContext); account = userAccountManager.getAccountByName(loginName + "@" + baseUrl); - if (account == null) { - throw new ActivityNotFoundException(); + if (Objects.equals(account.type, targetContext.getString(R.string.anonymous_account_type))) { + throw new RuntimeException("Could not get account with name " + loginName + "@" + baseUrl); } Optional optionalUser = userAccountManager.getUser(account.name); @@ -121,14 +127,19 @@ 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()); + var result = new ReadFolderRemoteOperation("/").execute(client); + assertTrue(result.getLogMessage(targetContext), result.isSuccess()); for (Object object : result.getData()) { RemoteFile remoteFile = (RemoteFile) object; - if (!remoteFile.getRemotePath().equals("/")) { + if (!Objects.equals(remoteFile.getRemotePath(), "/")) { if (remoteFile.isEncrypted()) { ToggleEncryptionRemoteOperation operation = new ToggleEncryptionRemoteOperation(remoteFile.getLocalId(), remoteFile.getRemotePath(), @@ -138,6 +149,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); } @@ -243,7 +261,7 @@ public boolean isPowerSavingEnabled() { newUpload.setRemoteFolderToBeCreated(); - RemoteOperationResult result = newUpload.execute(client); + var result = newUpload.execute(client); assertTrue(result.getLogMessage(), result.isSuccess()); OCFile parentFolder = getStorageManager() @@ -252,6 +270,7 @@ public boolean isPowerSavingEnabled() { OCFile uploadedFile = getStorageManager(). getFileByDecryptedRemotePath(parentFolder.getDecryptedRemotePath() + uploadedFileName); + assertNotNull(uploadedFile); assertNotNull(uploadedFile.getRemoteId()); assertNotNull(uploadedFile.getPermissions()); 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()); } diff --git a/app/src/androidTest/java/com/owncloud/android/ui/dialog/DialogFragmentIT.kt b/app/src/androidTest/java/com/owncloud/android/ui/dialog/DialogFragmentIT.kt index 9f950eac778a..4f039da6bf63 100644 --- a/app/src/androidTest/java/com/owncloud/android/ui/dialog/DialogFragmentIT.kt +++ b/app/src/androidTest/java/com/owncloud/android/ui/dialog/DialogFragmentIT.kt @@ -504,9 +504,8 @@ class DialogFragmentIT : AbstractIT() { throw UnsupportedOperationException("Document scan is not available") } - val materialSchemesProvider = getMaterialSchemesProvider() val viewThemeUtils = ViewThemeUtils( - materialSchemesProvider.getMaterialSchemesForCurrentUser(), + materialSchemesForCurrentUser, ColorUtil(targetContext) )