changes to run locally and misc fixes and/or refactoring

[mach6]

1. Client now uses selion enhanced grid for local runs
2. ios-driver 0.6.6-snapshot is gone but optional
3. Client has no selenium-sever dependencies unless running locally. Server dependencies for local runs are downloaded on demand and/or configured in by user
4. Misc other changes ... view commit message(s) for additional details

[krmahadevan]

abstract keywords are redundant in an interface. Please have them removed.

[mach6]

Yes. Sure are. Will fix

[krmahadevan]

Minor observation

We can remove implements LocalServerComponent from the below classes

• LocalHub
• LocalSelendroidNode
• LocalNode
• LocalIOSNode

since all of the above 4 classes extend from AbstractBaseLocalServerComponent and since the base class already implements LocalServerComponent

On a related observation, I believe some of the discussions we were having were related to getting rid of the need for LocalHub and LocalNode for browser based automation and instead directly start leveraging the existing concrete WebDriver implementations. Are those going to be a separate change ?

Also I believe that for starting a test locally against iOS driver (or) selendroid, we can very well just spawn the LocalIOSNode/ LocalSelendroidNode and then work with creating a RemoteWebDriver instance using it. If that is the route that we take then I believe we can very well get rid of the LocalHub and LocalNode classes for sure.

[krmahadevan]

Please fix the typo

[mach6]

Took me a min to see it, but yes, will fix it

[krmahadevan]

The current implementation causes problems for the very first time when someone wants to basically run a web test locally. This scenario becomes valid especially when the user is on a very slow network and the downloading is slow.

In my case phantomjs-1.9.8-macosx.zip took a long time to download and even before it could finish, I ended up triggering a

Caused by: java.lang.IllegalStateException: Timed out waiting for LocalNode to initialize.
    at com.paypal.selion.platform.grid.AbstractBaseLocalServerComponent.waitForInitialization(AbstractBaseLocalServerComponent.java:113)
    at com.paypal.selion.platform.grid.AbstractBaseLocalServerComponent.boot(AbstractBaseLocalServerComponent.java:63)
    ... 31 more

[krmahadevan]

@mach6

I feel that we may perhaps have complicated things a little bit here.
I was hoping that we could be doing just the following to get this done at the local executions level.

• Introduce 1 new Configuration property that would tell SeLion how to spin off a new server. This would be the command that we would be using to spin off either a Selendroid node (or) an Appium node (or) an iOS Driver node.
• Get rid of LocalHub, LocalNode etc., and just have 1 class named "LocalRemoteNode" (or whatever we would like to call it as ) which would spin off a child process by using the command that the user has specified and target @MobileTest annotated test runs against it.
• For @WebTest annotated test methods which are to be running locally, we would be falling back to using FirefoxDriver, InternetExplorerDriver, ChromeDriver etc., and thus do away with the need for using a programmatically spun Hub and Node.

That way we don't need to bring in Selion Grid as a dependency on the Client and leave things as they are.

[mach6]

I am having problems in getting the suite file suites/SelendroidMobileSuite.xml .....

This is a bug. I introduced the ability to bypass the timestamp check in FileDownloader by setting ProcessLauncherOptions .. Although the option setFileDownloadCheckTimeStampOnInvocation(false) is set in LocalSelendroidNode, it seems to not be happening.. So, I'll look into this..

[mach6]

Caused by: java.lang.IllegalStateException: Timed out waiting for LocalNode to initialize.

Letting initialization run indefinitely did not feel like the right thing to do. Thus, the initialization timeout is currently configurable via ConfigProperty.EXECUTION_TIMEOUT. Perhaps a a new config option would be more suitable. Another thought I had was to package these artifacts (the one that we would otherwise download via FileDownloader) in the SeLion jar and extract them (instead of downloading them) as long as the filename and checksum matches the content of the download.json file.

[krmahadevan]

@mach6
Here's the problem that I have with this :

Imagine the below scenario. Our user is working on an offline basis wherein he/she doesn't have an internet connection.
Now the user wants to basically run some functional tests against a locally running Tomcat server which hosts his/her web application for which the user wants to write automation tests.

Our current model will never let a user run his tests because it expects to download chromedriver/phantomJS driver/InternetExplorer driver etc., from the internet apart from downloading the selenium jars before he/she can run anything.

What if the user is behind a corporate proxy/firewall setup which prevents downloads from the internet ?

This to me is a big step in the reverse direction.

IMO we should delegate the responsibility of setting up the local environment to the user (even Selenium expects the users to do this as a pre-requisite and doesn't automagically do it for them) and just run against the setup environment.

Also if a user has already setup his environment (such as installing chromedriver and phantomJS driver via tools such as brew ) shouldn't we be honoring that setup rather than us try to setup things from the scratch ?

So for local runs we should just keep things simple by asking the user to provide us with the command to spin off a node and use that command to spin off a single sub process (ONLY for selendroid, iOS-driver and appium local runs use cases) and then have the tests run against that newly spun off sub process.

For web tests involving local runs we should use ChromeDriver, InternetExplorerDriver, FirefoxDriver etc., and do away with the local hub and local node.

[mach6]

@krmahadevan I fail to see how downloading dependencies (in the case of brew or other) which requires an internet connection and pre-work is any different -- a user can pre-establish ~/.selion, if needed. The means to specify alternative chromedriver, phantomjs, etc installations exists in the same fashion as it did prior to this change (we can and likely should do the same for ios-driver standalone and selenium-sever standalone). The only difference is by default, we are trying to setup the environment IFF those binary/artifact locations were not provided to us. The exception being selenium-server -- which MUST be included in the users pom (therefore already in the local maven repo) and should NOT (if it is, this was not the intent, and I need to fix it) be downloaded for a LocalHub orLocalNode when these are created via ThreadedLauncher

For web tests involving local runs we should use ChromeDriver, InternetExplorerDriver, FirefoxDriver etc., and do away with the local hub and local node...

This change, if we decide to go this route, can come later. My intent here was to address the multiple selenium-server version conflicts, ios-driver snapshot, multiple implementations (both in client and server) for spinning up Sel nodes/hubs etc and to keep the general approach of using RemoteWebDriver and a single access port (4444) everywhere in tact for now. In this effort we gained some things -- but, yes it does mean a couple new assumptions.

The changes I made to the "server" jar needed to happen regardless. We needed to support launching more than just a regular Sel node, hub, or standalone

[krmahadevan]

@mach6
I believe you are taking my example in the literal sense. This implementation assumes that the artifacts will be available in a pre-defined location. If its not available in that location it would be downloaded.

The point I am trying to make here is that, this assumption is an overhead due to the following reasons

• How does the user know about this assumption ? Yes documentation could be one of the things. But then again not all things can be taken care of in that manner. For e.g., a user would perhaps have already "pre-setup" his machine by making the chromedriver, phantomJs binaries in predefined locations of his/her choice. Why are we expecting that they be provided again in a different location ?
• If the machine was already pre-setup by the user in terms of making the jars available and the binaries being available I am of the opinion that we should just use them.

With respect to using other web drivers, I believe that it should be done in this itself because doing that is going to get rid of the need for a local Hub and local node completely. That means that there is no need for the changes done at the server jar to be made. The current implementation that we had (ie., a customized Grid with a plain vanilla node) itself should suffice. Launching more than just the regular node/hub and standalone is IMO not required because there is not much we are doing in the other standalones (such as appium, iOS driver, selendroid) apart from providing mechanisms for recycling a node via extra servlets.

[sebady]

I see some value in having the SeLion client use our local hub and a local node. This provides the ability to have an execution environment that is "closer" to what you will get from our Grid. We can also support a direct runlocally mode where the browser driver instances are leveraged directly (from a user configurable location if necessary). This is really a new capability though and I would recommend we try to support both modes of "runlocally".

[mach6]

This implementation assumes that the artifacts will be available in a pre-defined location. If its not available in that location it would be downloaded.

@krmahadevan this behavior is fixed in the most recent updates / commit. Driver binaries are downloaded to and used in SELION_HOME_DIR as a last resort. --- selenium is not downloaded anymore for LocalHub/LocalNode -- Also addressed your other misc feedback. Last, as we discussed offline (and for the reasons stated) this implementation continues to spin up a local hub and local node for @WebTest needs.

[mach6]

@fakrudeen78 @sebady @krmahadevan @shelar Ping.. New commits on this PR. Feedback welcome. Note: I still have pending (not yet complete) changes for the one area I see outstanding with this PR -- improving the AbstractBaseProcessLauncher#shutdown() logic

[krmahadevan]

@mach6
Can you please help elaborate as to what is going to be done with #115 ?

Without us making a decision on that, I don't think it makes sense for us to continue vetting out this delivery.

I am stil not convinced that we need such an elaborate implementation to support local execution.

[sundaramrajendran]

@mach6

I think we can remove the capability of starting the Grid locally in SeLion client which will simplify the code:

• Can remove code for starting the grid
• Get rid of browser driver config and SELENIUM_RUN_LOCALLY variables
• No need to download the Selenium-Server and Selion-Grid dependencies in client

Your approach simplify the user experience, but I think right now setting-up/starting our SeLion grid is very simple. So user can easily do it.

Just my thought on this...

[mach6]

W.R.T to running local for web, etc. My intention and approach in this PR keeps the existing capabilities intact as much as possible.

We can/should pursue other options after M5 and 1.0.0.