ANDROID-17023 Move Loggerazzi fixes to Snaptesting library#14
ANDROID-17023 Move Loggerazzi fixes to Snaptesting library#14DevPabloGarcia merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ports Loggerazzi fixes to the Snaptesting library, including improvements to log comparison, file filtering logic, and SDK version updates.
- Refactored file filtering to handle recorded and failure results together, with proper cleanup and renaming
- Added retry mechanism with timeout for log comparisons to handle asynchronous log recording
- Introduced
AnyOrderLogComparatorfor flexible log matching and updated SDK target to 36
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| AndroidSnaptestingPlugin.kt | Refactored file filtering logic to process recorded and failure directories together with improved cleanup |
| libs.versions.toml | Updated target and compile SDK versions from 35 to 36 |
| MainActivity.kt | Added edge-to-edge display support |
| LogsRule.kt | Introduced retry mechanism with 5-second timeout for log comparisons |
| LogComparator.kt | Added AnyOrderLogComparator class and fixed error message parameter order |
| README.md | Documented the 5-second timeout behavior for log comparisons |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lastRecordedFile | ||
| .renameTo(File(recordedDir, "$key.txt")) |
There was a problem hiding this comment.
The result of renameTo() should be checked. If the rename operation fails, the code continues silently without handling the failure, potentially leaving files in an inconsistent state. Consider checking the boolean return value and handling failures appropriately.
| lastRecordedFile | |
| .renameTo(File(recordedDir, "$key.txt")) | |
| val renamed = lastRecordedFile | |
| .renameTo(File(recordedDir, "$key.txt")) | |
| if (!renamed) { | |
| System.err.println("Failed to rename file: ${lastRecordedFile.absolutePath} to ${File(recordedDir, "$key.txt").absolutePath}") | |
| } |
| .renameTo(File(recordedDir, "$key.txt")) | ||
| File(failuresDir, lastRecordedFile.name) | ||
| .takeIf { it.exists() } | ||
| ?.renameTo(File(failuresDir, "$key.txt")) |
There was a problem hiding this comment.
The result of renameTo() should be checked. If the rename operation fails, the code continues silently without handling the failure, potentially leaving files in an inconsistent state. Consider checking the boolean return value and handling failures appropriately.
| val comparisonFailure = comparator.compare(recordedLogs, goldenLogs) | ||
| comparison = Comparison(comparisonFailure, recordedLogs) | ||
| if (!comparison.success) { | ||
| Thread.sleep(RESULT_POLLING_INTERVAL_MS) |
There was a problem hiding this comment.
Using Thread.sleep() on the instrumentation test thread can cause issues in Android testing. Consider using SystemClock.sleep() from android.os.SystemClock instead, which is the recommended approach for Android instrumentation tests.
🎟️ Jira ticket
ANDROID-17023
🥅 What's the goal?
We developed some fixes in Loggerazzi, we need to copy them in Snaptesting library
🚧 How do we do it?
📘 Documentation changes?
🧪 How can I test this?