Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add failed driver guard to FileManager Appium API #709

Merged
merged 8 commits into from
Jan 10, 2025

Conversation

twometresteve
Copy link
Contributor

@twometresteve twometresteve commented Dec 20, 2024

Goal

Add failed driver guard to FileManager Appium API.

Design

First of a number of PRs to shift logic away from Maze::Appium::Driver and protect clients from noise caused by a failed underlying Appium driver.

Tests

Unit test added and some local testing performed, but further testing will be performed on the integration branch prior to release.

@twometresteve twometresteve force-pushed the tms/file-manager-guard branch from 85eaa21 to d3773ed Compare January 8, 2025 17:42
@twometresteve twometresteve requested a review from Cawllec January 8, 2025 18:32
@@ -45,7 +61,15 @@ def read_app_file(filename, directory = nil)
end

$logger.trace "Attempting to read file from '#{path}'"
file = @driver.pull_file(path)
@driver.pull_file(path)
true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is attempting to retrieve a file this should be the file pulled rather than the success metric?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gosh, you're absolutely right - I'd assumed the file is written straight to file, but it is returned in memory by the method call. Good spot.

@twometresteve twometresteve requested a review from Cawllec January 9, 2025 14:57
@twometresteve twometresteve merged commit 7c4ccd9 into integration/appium-api Jan 10, 2025
25 checks passed
@twometresteve twometresteve deleted the tms/file-manager-guard branch January 10, 2025 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants