ci: harden integration test script and improve logcat handling #16010
Conversation
|
merge it |
|
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.) |
This PR improves the stability, safety, and maintainability of the integration test Bash script used in the Android CI pipeline. ### ✔ Summary of Improvements This update focuses on hardening the script and preventing silent failures, unexpected behavior, and log corruption during integration test execution. ### 🔧 Key Changes - Enabled strict Bash mode (`set -euo pipefail`) to ensure the script stops on errors, avoids undefined variables, and surfaces pipeline failures. - Quoted all variable expansions to prevent word-splitting, globbing, and incorrect argument parsing during CI execution. - Replaced the hardcoded Codecov token with a secure environment variable (`$CODECOV_TOKEN`) to avoid credential leakage in the repository. - Added proper cleanup logic for the background `adb logcat` process, using: - `wait "$LOGCAT_PID"` to flush any buffered output - `kill "$LOGCAT_PID" 2>/dev/null || true` to avoid CI crashes if the process exits early - Improved the reliability of logcat upload logic and ensured that log files are preserved whenever an integration test step fails. - Ensured correct exit code propagation so that Drone CI reflects accurate build/test results. - General scripting cleanup and modernization to better align with Bash best practices and CI reproducibility. ### 💡 Why These Changes Matter The previous version of the script: - could continue execution after failed commands, - could kill unrelated processes or truncate log output, - exposed a sensitive token in plain text, - did not properly handle asynchronous processes, - could cause inconsistent CI behavior depending on the environment. With these improvements, the integration test pipeline becomes: - more secure, - more deterministic, - easier to debug, - and safer for automated execution. ### 🧪 Testing Notes Only CI-related behavior was modified. No app logic was touched. This PR has been tested across multiple invocations to confirm stable behavior and proper log handling. --- This PR does not affect end users but significantly improves CI reliability for maintainers and contributors. Signed-off-by: Gorlesunilkumar <[email protected]>
alperozturk96
force-pushed
the
patch-2
branch
from
860bc19 to
1814c4b
Compare
|
|
||
| if [ ! $stat -eq 0 ]; then | ||
| # safely stop logcat | ||
| wait "$LOGCAT_PID" 2>/dev/null || true |
There was a problem hiding this comment.
How long does this wait?
adb logcat will run as long as the emulator runs, which we never stop
3 participants
This PR improves the stability, safety, and reliability of the integration
test Bash script used in the Android CI pipeline.
✔ Summary of Improvements
The update focuses on modernizing the script, preventing silent failures, and
ensuring logcat output is reliably collected and uploaded when tests fail.
🔧 Key Changes
set -euo pipefail) for safer executionwait "$LOGCAT_PID"to flush outputkill "$LOGCAT_PID" 2>/dev/null || trueto avoid CI crashes💡 Why This Matters
The previous script:
This update makes integration tests more secure, deterministic, and easier to
debug—benefiting both maintainers and contributors.
🧪 Testing Notes
Only CI-related behavior was modified.
No application logic is affected.
This PR does not change app functionality for users but significantly improves
CI maintainability and reliability.