Repository navigation
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🟡 Changes recommended
The new file-verification steps lack user documentation, and tests do not cover the alternative missing-file error or the accessibility guard that determines whether read failures are propagated.
2 open issues
What changed in this PR
This PR aims to stabilize Android SauceLabs system tests by checking device files directly instead of relying on Photos indexing, and by adjusting carousel gestures.
Changes:
- Add steps to compare device-file bytes with a resource and verify file absence.
- Update Android upload and deletion scenarios to use these assertions.
- Use coordinate-based carousel drags and add unit tests for file verification.
| File | Description |
|---|---|
vividus-tests/src/main/resources/story/system/mobile_app/MobileAppStepsTests.story |
Replaces Photos checks with file assertions and adjusts carousel drags. |
vividus-plugin-mobile-app/src/test/java/org/vividus/mobileapp/steps/DeviceStepsTests.java |
Tests the new file-verification steps. |
vividus-plugin-mobile-app/src/test/java/org/vividus/mobileapp/action/DeviceActionsTests.java |
Tests file existence checks and unexpected-error propagation. |
vividus-plugin-mobile-app/src/main/java/org/vividus/mobileapp/steps/DeviceSteps.java |
Adds resource-content comparison and file-absence assertions. |
vividus-plugin-mobile-app/src/main/java/org/vividus/mobileapp/action/DeviceActions.java |
Adds file existence detection using file retrieval and missing-file errors. |
Review effort: Balanced
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7036 +/- ##
=========================================
Coverage 97.83% 97.84%
- Complexity 7434 7448 +14
=========================================
Files 1028 1028
Lines 21916 21955 +39
Branches 1451 1458 +7
=========================================
+ Hits 21442 21481 +39
Misses 357 357
Partials 117 117 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Qodana for JVM2 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
13666e0 to
56238c9
Compare
a7fcc0e to
eeaa081
Compare
There was a problem hiding this comment.
🟡 Changes recommended
A moderate active issue remains because NOT_EQUAL_TO bypasses byte-array content comparison and incorrectly relies on identity strings, requiring a fix to comparison and soft-assert behavior.
1 open issue
1 resolved since last review
Review effort: Lite
| if (variable1 instanceof byte[] bytes1 && variable2 instanceof byte[] bytes2) | ||
| { | ||
| return compareBinary(bytes1, bytes2); | ||
| } |
There was a problem hiding this comment.
Binary byte-array comparisons now use content equality for NOT_EQUAL_TO as well, with the corresponding assertion outcome/message and regression tests. Commit: 065f9c931.
Co-authored-by: valfirst <5081226+valfirst@users.noreply.github.com>
CI failed: Android and iOS SauceLabs system test scenarios failed while creating Appium sessions because Sauce Labs reported infrastructure errors preparing or starting devices; the build failure is a downstream result and the PR changes do not appear to cause it (one finding, unrelated, infrastructure).OverviewTwo unique error templates were found across 28 analyzed logs for commit 065f9c9. Both point to Sauce Labs / device session-creation failures during the mobile system test run, and the analyzer judged them unrelated to the PR changes. FailuresSauce Labs session creation failed with infrastructure errors (confidence: high)
Summary
Code Review ✅ Approved 1 closed / 1 findings🟡 Medium risk · Binary comparisons now use byte-content equality, and device-file checks infer absence from driver errors; misclassified errors could make system tests report files missing. Adds byte-level comparison for ✅ 1 closed✅ Performance: Byte-array comparison puts multi-MB decimal dumps into report
VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Approved For more verification details, comment Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| protected boolean compareBinary(byte[] bytes1, ComparisonRule condition, byte[] bytes2) | ||
| { | ||
| boolean equal = Arrays.equals(bytes1, bytes2); | ||
| return condition == ComparisonRule.EQUAL_TO ? equal : !equal; |
| protected boolean compareBinary(byte[] bytes1, ComparisonRule condition, byte[] bytes2) | ||
| { | ||
| boolean equal = super.compareBinary(bytes1, ComparisonRule.EQUAL_TO, bytes2); | ||
| boolean passed = condition == ComparisonRule.EQUAL_TO ? equal : !equal; |
|
Summary
Validation
Summary by Gitar
byte[]variables by content inVariableComparatorandVariablesStepsThen file ... does not exist on devicebacked byDeviceActions.fileExistsplugin-mobile-app.adocThis will update automatically on new commits.