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 retry logic to findElement. #33

Merged
merged 5 commits into from
Jul 24, 2023
Merged

Add retry logic to findElement. #33

merged 5 commits into from
Jul 24, 2023

Conversation

jeffdav
Copy link
Contributor

@jeffdav jeffdav commented Jul 20, 2023

  • Added maxRetries and retryInterval to session object in case anyone wants to change defaults.
  • Added withRetry = true param to all the various public findElement methods.
  • Added withRetry without default param to the internal findElement method that does the actual work so future overloads don't accidentally forget to pass the value through.
  • Changed private findElement workhorse to internal and unified the Element.findElement methods to call the single implementation in Session. The only difference between the two code paths was the eventual URL that got constructed, and this removes a lot of duplicate code.
  • Made a few error messages more clear.

Fixes WIN-496

@jeffdav
Copy link
Contributor Author

jeffdav commented Jul 20, 2023

(I'm not sure this autolink thing is working.)

@tristanlabelle
Copy link
Contributor

(I'm not sure this autolink thing is working.)

Maybe it's not setup on this particular repo 🤔

Copy link
Contributor

@tristanlabelle tristanlabelle left a comment

Choose a reason for hiding this comment

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

The structure of the code is looking great but I think should use a different API shape.

Sources/Element+Request.swift Outdated Show resolved Hide resolved
Sources/Session.swift Outdated Show resolved Hide resolved
Tests/WebDriverTests/NotepadTests.swift Outdated Show resolved Hide resolved
Sources/Session+Requests.swift Outdated Show resolved Hide resolved
Sources/Session+Requests.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@tristanlabelle tristanlabelle left a comment

Choose a reason for hiding this comment

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

A couple more comments but I think this is the functionality we want! Nice work

Sources/Retry.swift Show resolved Hide resolved
Sources/Retry.swift Show resolved Hide resolved
Sources/Retry.swift Show resolved Hide resolved
Sources/Retry.swift Show resolved Hide resolved
@jeffdav jeffdav merged commit ed25c1f into main Jul 24, 2023
1 check passed
@jeffdav jeffdav deleted the retry branch July 24, 2023 04:38
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