-
Notifications
You must be signed in to change notification settings - Fork 4
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
Skip remaining scenarios if the Appium session fails #708
Conversation
lib/maze.rb
Outdated
:public_document_server_address, :run_uuid, :scenario | ||
|
||
def config | ||
@config ||= Maze::Configuration.new | ||
end | ||
|
||
def driver | ||
raise 'Cannot use a failed driver' if @driver&.failed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this raise cause each attempt to acquire the driver just raise an error when called, but not actually return anything for logical checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So trying to call Maze.driver.failed?
will simply raise the error and not actually return the driver, so it won't trigger checks like if Maze.driver.failed?
.
Could we simply log instead, but allow the driver to be returned and checked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So trying to call Maze.driver.failed? will simply raise the error
Oh wow, yeah, that's not a good plan - good spot. With hindsight, I think doing anything at this level is a bad idea. The next change I'm working on will add an API layer above the driver, which will handle use of a failed driver. So I think for this change we leave it with driver
as a simple attr_accessor
.
Goal
Skip remaining scenarios if the Appium session fails.
Design
Previously we were nilling out
Maze.driver
when it failed and guarding any uses of it with the&.
. That was generally ok, but the problem with&.
is that operations essentially fail silently. This change introduces an explicit failed state, which we can build more rigour on top of.Changeset
Tests
This change can be seen in action on this trial build:
https://buildkite.com/bugsnag/bugsnag-unity/builds/5355#0193cf49-9bcd-4d1a-adad-2d719f96ea89
Although there are many errors in amongst the skipped scenarios, they are due to the failed driver still attempting to be used. I've raised PLAT-13337 to introduce a formal API for accessing driver methods instead of calling it directly.