willowtreeapps / conductor-mobile

Conductor Mobile is a port of the Conductor Web Framework for iOS and Android
Apache License 2.0
6 stars 5 forks source link

[FEATURE] Decouple unnecessary functionality from Conductor Mobile #119

Open mrk-han opened 5 years ago

mrk-han commented 5 years ago

Is your feature request related to a problem? Please describe. Conductor Mobile has a lot of responsibilities and makes many assumptions/decisions that should be controlled by user.

Examples

Describe the solution you'd like

Research into and report on ways to make Conductor into a better framework for managing Configurations / Schemas and having utility functions for certain things, and not being too controlling on what we force the project to do.

Additional context @jossjacobo once we get a better idea for what can be done, we can break this into more issues if needed

jossjacobo commented 5 years ago

Here are my initial thoughts:

Sample New Implementation

public class SettingsTest {

    Conductor conductor;

    @BeforeClass
    public void init() {
        ConductorConfig conductorConfig = new ConductorConfig();
        this.conductor =  Conductor.getInstance(conductorConfig);
        this.conductor.startAppiumServer();

        // Alternatively they'd be able to call something like this, to only 
        // leverage the `config.yaml` file, and pass those caps into their own
        // appium server implementation
        DesiredCapabilities caps = conductorConfig.buildCapabilities();
    }

    @Test
    public void normalTest() {
      // Normal conductor Page Object Model Navigation
      conductor.click(By.id("menu"))
         .click("settings");

      // New validation methods
       Validate.with(conductor)
         .that(By.className("title"))
         .isEqualTo("Bleg")
         .andThat(By.cssSelector(".list-item[qa-automation='blob']")
         .isPreset()
         .andThat(errorMessage)
         .isNotPresent()
         .andThat(...) // etc.

       // Alternatively they can also use whatever assertion library they'd like
    }

    @AfterClass
    public void tearDown() {
        this.conductor.stopAppiumServer();
    }
}

Would love some thoughts on this approach @mrk-han @evant @konwinkler or anyone else.

Thanks,

evant commented 5 years ago

Since we have some projects using conductor pretty heavily, I think it's worth having a compatibility period when making these changes. ie keep the old classes around deprecated. One thing we can do is split the testing stuff out into a separate artifact so you only need to depend on junit/testng if you need those 'legacy' classes.

jossjacobo commented 5 years ago

Yup, that's what I was planning on doing. Was going to break up the functionality into the new APIs but still keep the older APIs with a @deprecated tag.

What do you mean by building separate artifacts?

mrk-han commented 5 years ago

What are the goals of the validations that we provide with Conductor? Do we want to continue to provide assertion utilities?

On the pro-side, we do use a few of them heavily in our Project Repo and have proven useful, and it could be cool to make a DSL syntax for Conductor Validations with the Validate class. And perhaps having a softAssertAllOf or just an assertAllOf utility method would be nice!

Negatively, it can lead to an inconsistency in using them vs. another assertion library which leads to confusion. And may require more work/maintenance/testing to be done on the Conductor Library.

aaron-goff commented 5 years ago

@mrk-han I think we could get by refactoring out those usages of the validation methods.

I think I'm in favor of removing/simplifying the validation methods. I'm not entirely sure the work -> reward of having them is there? There are other libraries out there that do what we'd be trying to accomplish and are better maintained/more fully formed.

evant commented 5 years ago

@jossjacobo Something like

com.willowtreeapps:conductor-mobile
- io.appium:java-client
- ...

com.willowtreeapps:conductor-mobile-legacy
- com.willowtreeapps:conductor-mobile
- org.testng:testng
- org.junit.platform:junit-platform-surefire-provider
- org.assertj:assertj-swing-junit

It allows us to keep the deprecated classes around while still getting rid of the deps for someone not using them.

jossjacobo commented 5 years ago

Oh I see what you mean, so the new upgrade path for people not wanting to do the migration just yet will be to just add another dependency. Yeah that would work pretty cleanly 👍

mrk-han commented 5 years ago

Could be good to look at what https://github.com/SchibstedSpain/Barista does for inspiration, and to see if our goals align, or what we want to avoid doing that they do

Barista makes developing UI test faster, easier and more predictable. Built on top of Espresso, it provides a simple and discoverable API, removing most of the boilerplate and verbosity of common Espresso tasks. You and your Android team will write tests with no effort.