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

WIP for mobile support #147

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

WIP for mobile support #147

wants to merge 10 commits into from

Conversation

titusfortner
Copy link
Collaborator

@titusfortner titusfortner commented Apr 13, 2020

This is mostly working, but not yet ready for prime time;

Working code with specs:

  • EmuSim Browser
  • EmuSim App
  • Test Object Browser

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.

import java.net.MalformedURLException;
import java.net.URL;

public class SauceMobileSession extends SauceSession {
Copy link
Contributor

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?

Copy link
Collaborator Author

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.

@nadvolod
Copy link
Collaborator

Codacy Here is an overview of what got changed by this pull request:

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")) {
Copy link
Collaborator

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

public AppiumDriver start() {
MutableCapabilities capabilities = sauceOptions.toCapabilities(getDataCenter());

URL url;
Copy link
Collaborator

Choose a reason for hiding this comment

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

String browserName = sauceOptions.toCapabilities().getBrowserName();

if (browserName.equals("")){
if (environment.toLowerCase().equals("android")){
Copy link
Collaborator

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;
Copy link
Collaborator

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.*;
Copy link
Collaborator

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("")){
Copy link
Collaborator

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;
Copy link
Collaborator

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@titusfortner
Copy link
Collaborator Author

And... we can't authenticate via options, so we'll have to pull out that code... :(

@titusfortner titusfortner mentioned this pull request Jan 23, 2021
Base automatically changed from master to main March 15, 2021 20:20
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.

None yet

3 participants