Minor improvements for automated testing #15424
Conversation
PhilLab
commented
Aug 24, 2025
PhilLab
commented
- Minor improvements of testing documentation
- Instrumentation tests now automatically grab the notifications permission
- Tests written, or not not needed
CONTRIBUTING.md
Outdated
| 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: | ||
| - Start the server | ||
| - TODO: find out how... is it maybe this command: `docker run --name=uiComparison nextcloudci/server --entrypoint '/usr/local/bin/initnc.sh' 1>/dev/null` |
There was a problem hiding this comment.
@alperozturk96 If somebody could point me to the right direction how to develop and test locally any integration test which needs a reachable backend, I would like to explain it here. And how to get new test data, e.g. new expected remote files onto it.
Secondly, if there is an existing method of using a mocked server instead, please let me know and I can write it up here as well.
There was a problem hiding this comment.
You can run any server you like.
Best is to look into https://docs.nextcloud.com/server/latest/developer_manual/getting_started/devenv.html
There is a docker instruction, but also the "plain" php way described.
You can also use your real server, with a separate(!) user for testing.
Secondly, if there is an existing method of using a mocked server instead, please let me know and I can write it up here as well.
There are some tests, which do not require a server. eg. FileDetailFragmentStaticServerIT or tests using TestActivity
There was a problem hiding this comment.
@tobiasKaminsky thank you for your response. What is now missing for me is how I can set up such a server-based test, test it locally, and then have it executed as part of the regular Github CI.
For it to work on the CI, it would require that I run a server in a similar configuration locally (or use a separate test user on a real server, as you said) - but for both I need to know how to replicate on my own what will later be executed in the CI. That's the essence of my question.
There was a problem hiding this comment.
@tobiasKaminsky how can I set up such an end-to-end integration test which uses a specific state on the server, e.g. a specific list of files to download, etc...? Is there an existing test doing such a thing, e.g. by first uploading some files in an @Before test method? My aim is to educate (and be educated) how to extend the test suite.
If this is nothing which exists already, I'd remove this readme section from the PR and have the rest merged, as it would already be a small improvement.
There was a problem hiding this comment.
@tobiasKaminsky okay, I sorted it out and added explanations to the markdown file as well as the abstract test class.
Can you kindly let me know how and when these AbstractOnServerIT run in the CI? What I think right now:
- AbstractOnServerIT tests are run as part of runCombinedTest.sh
- That script is called in .drone.yml
- Drone CI executes them after the merge. You can access the results in https://drone.nextcloud.com
- The status of master is continuously monitored and if tests start to fail, a new PR is created to fix the code (or the tests)
However, where I need help:
- when I tried to find runs of a specific test
DownloadITto verify my assumptions, I failed to find them. - I was wondering if there is an existing mechanism to run these tests before broken code is merged?
- How do I find the Drone run for a specific merge? I clicked through the activity feed to find my PR, which was tedious, especially if the PR is older
- Verifying my PR
#15669I found https://drone.nextcloud.com/nextcloud/android/25363 , but no tests ran there. Why? What are the controls there?
If you are wondering, I am trying to come up with automated tests for PR #15669 and document what I am learning in the process
There was a problem hiding this comment.
@tobiasKaminsky @alperozturk96 can you take a look? This shouldn't be much effort. If anything, we could merge this already as it is, I think it would be a no-brainer improvement and help other developers
PhilLab
changed the title
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
PhilLab
force-pushed
the
automated_testing_minor_improvements
branch
2 times, most recently
from
14b716d to
38fa3da
Compare
- Fixed instructions for running tests. The mentioned example test had been refactored so the given commands were not able to locate a test to run. - Fixed copy/paste errors in test comments - Documented how to write server-based tests Signed-off-by: Philipp Hasper <[email protected]>
…sion Before that, when starting individual tests from the command line or from inside the IDE, they could fail because a dialog asking for the permission to post notifications was blocking the view. While we are on it, added a small explanation to the other existing rule. Without that explanation it might be unclear why this is not also done via the same GrantPermissionRule used for the notifications. Signed-off-by: Philipp Hasper <[email protected]>
Signed-off-by: Philipp Hasper <[email protected]>
The account type depends on the build flavor, as some of them define their own R.string.account_type. The test did respect that value when creating the account in AbstractIT.createAccount(), but not when deleting the account beforehand. Signed-off-by: Philipp Hasper <[email protected]>
TODO: helper function to check mime type for folder should probably move to the RemoteFile class in the Nextcloud Library. Signed-off-by: Philipp Hasper <[email protected]>
PhilLab
force-pushed
the
automated_testing_minor_improvements
branch
from
38fa3da to
5096dea
Compare
| * Hot fixes not relevant for an upcoming feature release but the latest release can target the bug fix branch directly | ||
|
|
||
|
|
||
| ### Android Studio formatter setup |
There was a problem hiding this comment.
@tobiasKaminsky regarding the formatting I also needed help on my previous PR. Shall I add the gradlew commands here? I notice that I keep looking for them again and again.
### Formatter setup
Our formatter setup is rather simple:
* Standard Android Studio
* 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:
./gradlew spotlessApply
./gradlew detekt
./gradlew spotlessCheck
./gradlew spotlessKotlinCheck
``
2 participants
