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

Appium Architecture Overhaul #5169

Closed
39 of 43 tasks
jlipps opened this issue May 27, 2015 · 4 comments
Closed
39 of 43 tasks

Appium Architecture Overhaul #5169

jlipps opened this issue May 27, 2015 · 4 comments
Milestone

Comments

@jlipps
Copy link
Member

jlipps commented May 27, 2015

The Problem

Over the years, Appium has grown organically in sometimes not-so-great ways. We've identified a number of problems that make Appium increasingly difficult to work on (especially for new developers) and increasingly prone to introducing regressions:

  1. Code is not modular enough—this makes testing independent chunks difficult. Example: the Java bootstrap is really its own project and could be made into its own NPM-wrapped Maven project and tested independently.
  2. Code is too modular in bad ways. Example: ios.js and ios-controller.js define methods for the same class but in different files.
  3. ES5-style JS has a number of gotchas that make it difficult for contributors to get started and require a strong policing effort on the part of reviewers. Despite a well-defined style guide, there were still too many ways to possibly do one thing. Examples:
    1. a confusing mix of doing OO-style programming with Prototypal inheritance
    2. having to annotate many anonymous functions with .bind(this) because of JS scoping
    3. confusion around whether to use for loops or _.each or Array.each
    4. mixing of functional and OO paradigms
  4. ES5-style JS requires use of either callbacks or promises to deal with the asynchronicity inherent in the Node library methods that are the foundation of Appium. Both of these options lead to confusion and are our primary source of programming errors in the form of race conditions, stalls, crashes, and multiple unintended execution paths. Asynchronous programming is hard for would-be contributors to wrap their minds around. There is confusion around whether to implement asynchronous control flow logic in raw JS or whether to use a library like caolan/async or Q.
  5. Appium uses too many metaphors for controlling subprocesses. We reinvent the wheel for every long-lived subprocess and have different levels of support for the various issues that come up with subprocesses (failure to start successfully, event handling based on stdout/stderr, unexpected process exit, killing processes with different signals, etc...). We need one model for dealing with Instruments, Chromedriver, UiAutomator, etc...
  6. There is not enough abstraction around the different "drivers" we support, which makes Appium less than ideally extensible if we want to add support for a new "driver" (such as DroidDriver, FirefoxOS Marionette, etc...)
  7. We reimplement understanding and handling of JSONWP concepts at multiple levels of the application; logic for this should all be concentrated in one place.
  8. There is too tight a coupling between a user's request object (ostensibly coming in at the "highest" level of Appium) and lower levels of Appium.
  9. Error handling is duplicated up and down the execution chain
  10. The test suite is poorly organized. It is organized by platform and test app instead of by feature. It tests too much (some tests should be unit tests) and too little (not every environment is tested, not every feature is tested, not every code path is tested. We don't have coverage reports in place to even know what level of coverage we have.
  11. The test apps are poorly suited in some cases to the features we want to test.
  12. Appium's build process takes place through an overgrown Bash script, and it rebuilds more than it needs to, rather than trusting subprojects to be ready to go. We have a mix of npm, Bash, Grunt, and Gulp instead of one build tool.
  13. Without a CI system running builds on every commit and pull requests, it's easy for regressions to slip through.

It's time to spend some effort and resources cleaning house and taking Appium to the next level of quality and longevity. Tackling tech debt is always a tradeoff, but we are reaching a point where developing features has already significantly slowed because of time spent fixing bugs, many of which are due to the tech debt we have accrued. It's worth the investment to resolve these issues now before we wind up in too much of a corner. Appium has been gaining significant traction in the marketplace, and appears to be functioning well for its current users. It's a good time to pause current development and prepare for the future. If we don't do this, Appium will lose the ability to develop at a velocity appropriate for its popularity and importance. The mobile strategy for many companies banks on Appium being stable, reliable, and cutting-edge. We don't want to lose that possibility because of tech debt. From a positive perspective, the Appium dev team will be one of the first to implement solutions to these kinds of problems for large Node.js apps. We'll reaffirm Appium's position on the cutting edge and have the opportunity to share our experiences and any wisdom we've gleaned from the process with other developers.

The Solution

We want to address these issues in the following way:

  1. Identify and break out bits of the software which can live as independent modules
    1. Design sensible interfaces for these which are agnostic to how they will be used in the larger software and thus have the benefit of being generally useful.
    2. Get to a high degree of test coverage, with an emphasis on unit tests and quick functional tests
    3. Put CI and semantic versioning in place so we never have the same regression twice in these modules
  2. Replace ES5 with ES6 (now called ES2015) everywhere. This will involve creating an ES6 style guide and standardizing approaches and patterns that we find are the best fit for our use case. This is where we'll pay attention to rethinking our overall architecture and how the bits are related to each other. We'll keep an eye out for opportunities to refactor and separate concerns, in conjunction with the first task above.
  3. Replace all asynchronous methods with async/await equivalents. async/await are new keywords proposed to enter JS in a future version. While not part of ES2015, they are already supported by transpilers and have a large fan-base. They are the equivalent of monocle-js but are supported as syntactic sugar over Promises, and are therefore to be preferred as part of the language itself. Moreover, they are semantically more transparent than using generators and yield. This will be a large task but will make Appium's code more compact and infinitely easier to read and write.
  4. Address the various issues listed in the 'Problem' section above as we come across them.

(Note that much of this strategy was worked out over a long period of time in #3911)

The Project

We want to do this work as part of the Appium 1.5 release. This release will target no features or bugfixes (other than ones that come along for free with the rearchitecture). The rationale for this is that we want to replace the guts of Appium with no visible change to the user, and no breaking changes. The test suite as it stands now should continue to pass after the rearchitecture. Once this is all in place, it will be much easier to fix bugs and add fixtures with greater velocity. We're talking about the most radical change to the Appium codebase that's happened since we moved to Node.js, but we want it to go completely unnoticed by users (unless they notice greater stability and reliability).

The Work

Here are the tasks which are a planned part of this rewrite, separated into 5 distinct phases:

Phase 1: Planning, new architecture, CI

  • plan the new Appium architecture
  • define a general CI strategy that can be used for both iOS and Android
  • create a Windows-based CI system to unit-test the JS code on Windows
  • create a package that encapsulates all the (mobile) JSON wire protocol logic and behavior
  • create an appium-specific express server package
  • create a base driver class that all appium drivers will extend
  • migrate capabilities parsing and validation to the base driver
  • create a fake driver for appium (used to demonstrate driver organization and for client tests)
  • update appium-logger with the ability for packages to define log prefixes

Phase 2: Migrating libraries

  • rewrite appium-instruments in es6+
  • rewrite appium-uiauto (the command proxy part) in es6+
  • rewrite appium-adb in es6+
  • create new package appium-uiautomator (from uiautomator.js)
  • create new package appium-remote-debugger (from remote-debugger.js and webkit-remote-debugger.js)
  • create new package appium-cookies (from cookies.js)
  • create new package appium-? from (iOS) settings.js and simulator.js

Phase 3: Migrating drivers

  • turn the Android Bootstrap into its own Java project
  • create appium-android-driver (wrap up the Bootstrap, appium-uiautomator, appium-adb, etc...)
  • create appium-ios-driver (wrap up appium-instruments, appium-uiauto, appium-xcode, etc...)
    • build or install or download ios-webkit-debug-proxy rather than requiring user to
    • figure out how to better bundle and sign SafariLauncher on install
    • standardize on one iOS real device library
  • create appium-selendroid-driver
    • download a pre-built selendroid server rather than building our own

Phase 4: Bringing it all together, cleanup, misc

  • rewrite appium as a much smaller "appium core" server that simply brings everything else together
  • deprecate command-line flags that are the same as caps, in favor of a --desired-capabilities flag
  • ensure that all sync (blocking) methods have been removed from all projects
  • ensure that all projects are using appium-logger
  • ensure that all instances of child_process.exec are now using teen_process.exec
  • ensure that all writes are done to temp dirs
  • ensure that all uses of temp dirs come from the same method
  • ensure that all binaries from subpackages are precompiled and available for the correct platform on npm install
  • develop a useful package management tool that can:
    • determine the status of an NPM dependency tree
    • check to see if an NPM project has unpublished commits
    • check to see if an NPM project has any unused deps
  • update NPM deps for all projects
  • clean up Winston logger code so that its only purpose is to convert npmlog events into output

Phase 5: Test, beta, release

  • pass the existing Appium testsuite against the new Appium
  • ensure that the new Appium can be built into the GUIs
  • ensure the Appium client testsuites pass against the new Appium
  • release 1.5 beta for public feedback
  • release 1.5 official after an appropriate waiting period
@jlipps jlipps added this to the Appium 1.5 milestone May 27, 2015
@triager triager added the Needs Triage bugs which are not yet confirmed label May 27, 2015
@jlipps jlipps removed the Needs Triage bugs which are not yet confirmed label May 27, 2015
@Jonahss
Copy link
Member

Jonahss commented May 27, 2015

Should this be a milestone?
edit: nvm, I see it's assigned to 1.5

@SrinivasanTarget
Copy link
Member

@jlipps Can we close this please?

@jlipps
Copy link
Member Author

jlipps commented Apr 8, 2016

yes, it's done!

@jlipps jlipps closed this as completed Apr 8, 2016
@lock
Copy link

lock bot commented Apr 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants