-
Notifications
You must be signed in to change notification settings - Fork 16
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
WIP for mobile support #147
base: main
Are you sure you want to change the base?
Conversation
…pport jwp-only endpoints
import java.net.MalformedURLException; | ||
import java.net.URL; | ||
|
||
public class SauceMobileSession extends SauceSession { |
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.
Not sure how I feel about Dividing SauceSession
and SauceMobileSession
. Can we combine the logic here into SauceSession
and have a single session
concept?
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.
We need some way of differentiating Appium Driver from Selenium Driver type. I started to look into doing generics and interfaces, but I can't tell if that's actually better for users.
Issues
======
- Added 9
Complexity increasing per file
==============================
- java/src/main/java/com/saucelabs/saucebindings/SauceMobileSession.java 3
- java/src/main/java/com/saucelabs/saucebindings/DeviceOrientation.java 1
- java/src/test/java/com/saucelabs/saucebindings/acceptance/SauceDesktopTest.java 1
- java/src/main/java/com/saucelabs/saucebindings/SauceAndroidOptions.java 1
- java/src/test/java/com/saucelabs/saucebindings/SauceAndroidOptionsTest.java 1
- java/src/main/java/com/saucelabs/saucebindings/SauceSession.java 1
- java/src/main/java/com/saucelabs/saucebindings/SauceMobileOptions.java 4
- java/src/test/java/com/saucelabs/saucebindings/acceptance/SauceRealDeviceBrowserTest.java 1
- java/src/main/java/com/saucelabs/saucebindings/SauceIOSOptions.java 1
- java/src/test/java/com/saucelabs/saucebindings/acceptance/SauceEmuSimBrowserTest.java 1
- java/src/test/java/com/saucelabs/saucebindings/SauceIOSOptionsTest.java 1
Clones added
============
- java/src/test/java/com/saucelabs/saucebindings/SauceOptionsTest.java 1
See the complete overview on Codacy |
if (environment.toLowerCase().equals("android")){ | ||
return createAndroidDriver(getSauceUrl(), sauceOptions.toCapabilities()); | ||
} | ||
else if (environment.toLowerCase().equals("ios")) { |
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.
import com.saucelabs.saucebindings.SauceMobileOptions; | ||
import com.saucelabs.saucebindings.SauceMobileSession; | ||
import com.saucelabs.saucebindings.SauceSession; | ||
import io.appium.java_client.AppiumDriver; |
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.
Issue found: Unused import - io.appium.java_client.AppiumDriver.
public AppiumDriver start() { | ||
MutableCapabilities capabilities = sauceOptions.toCapabilities(getDataCenter()); | ||
|
||
URL url; |
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.
Issue found: Avoid unused local variables such as 'url'.
String browserName = sauceOptions.toCapabilities().getBrowserName(); | ||
|
||
if (browserName.equals("")){ | ||
if (environment.toLowerCase().equals("android")){ |
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.
import org.junit.After; | ||
import org.junit.Ignore; | ||
import org.junit.Test; | ||
import org.openqa.selenium.remote.RemoteWebDriver; |
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.
@@ -0,0 +1,56 @@ | |||
package com.saucelabs.saucebindings.acceptance; | |||
|
|||
import com.saucelabs.saucebindings.*; |
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.
String environment = sauceOptions.toCapabilities().getCapability("platformName").toString(); | ||
String browserName = sauceOptions.toCapabilities().getBrowserName(); | ||
|
||
if (browserName.equals("")){ |
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.
Issue found: Position literals first in String comparisons
import com.saucelabs.saucebindings.SauceMobileOptions; | ||
import com.saucelabs.saucebindings.SauceMobileSession; | ||
import com.saucelabs.saucebindings.SauceSession; | ||
import io.appium.java_client.AppiumDriver; |
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.
import org.junit.After; | ||
import org.junit.Ignore; | ||
import org.junit.Test; | ||
import org.openqa.selenium.remote.RemoteWebDriver; |
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.
And... we can't authenticate via options, so we'll have to pull out that code... :( |
This is mostly working, but not yet ready for prime time;
Working code with specs:
It's going to need more unit tests for the options and ideally a new release of Appium bindings.
Feedback welcome.
Also, note that this works around the issue of most of our mobile endpoints not yet supporting w3c.
So this "works" but does not fulfill the goal of the bindings to generate future-compliant code. That said, this is why bindings are a good idea in that we can toggle over to generate the "correct" syntax as soon as Sauce supports it.