Simplified the StartsActivity by reducing the number of parameters through POJO class #579
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| package io.appium.java_client.android; | ||
|
|
||
| /** | ||
| * This is a simple POJO class to support the {@link StartsActivity}. | ||
| */ | ||
| public class Activity { | ||
|
There was a problem hiding this comment. I this this POJO should have 2 mandatory parameteres: package and activity. Please add the constructor with these parameters. |
||
| private String appPackage; | ||
| private String appActivity; | ||
| private String appWaitPackage; | ||
| private String appWaitActivity; | ||
| private String intentAction; | ||
| private String intentCategory; | ||
| private String intentFlags; | ||
| private String optionalIntentArguments; | ||
| private boolean stopApp; | ||
|
|
||
| /** | ||
| * A constructor with two mandatory parameters. | ||
| * | ||
| * @param appPackage The value for the app package. | ||
| * @param appActivity The value for the app activity. | ||
| */ | ||
| public Activity(String appPackage, String appActivity) { | ||
|
There was a problem hiding this comment. @email2vimalraj Could you provide these checkings: import static com.google.common.base.Preconditions.checkArgument;
import static org.apache.commons.lang3.StringUtils.isBlank;
...
public Activity(String appPackage, String appActivity) {
checkArgument(!isBlank(appPackage), "App package should be defined as not empty or null string");
checkArgument(!isBlank(appActivity), "App activity should be defined as not empty or null string");
...
} |
||
| this.appPackage = appPackage; | ||
| this.appActivity = appActivity; | ||
| this.stopApp = true; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the app package value. | ||
| * | ||
| * @return The app package value. | ||
| */ | ||
| public String getAppPackage() { | ||
| return appPackage; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the app package value. | ||
| * | ||
| * @param appPackage The app package value. | ||
| */ | ||
| public void setAppPackage(String appPackage) { | ||
|
There was a problem hiding this comment. I like the idea behind this but have few things to be discussed here, Is there a way to remove getters & setters? May be we need to come up with either custom annotation or something like https://github.com/rzwitserloot/lombok. May be not necessarily in this PR but subjective to discussion if there is any scope of improvement. There was a problem hiding this comment. Read the user review who was using the lombok for more than a year: http://stackoverflow.com/a/12807937/1505987 I would recommend to stay away from lombok, even though I never used it personally. My justification would be obviously a javadoc and everytime we will have to generate the code. There was a problem hiding this comment. I also don't recommend lombock. But i liked their way of using annotations. There was a problem hiding this comment. I'm not sure that it is necessary to keep this method. Maybe it is more senseful to keep only getter for package and make this field final. @SrinivasanTarget What is your opinion? There was a problem hiding this comment. +1 - Making this final would be a better option 👍 |
||
| this.appPackage = appPackage; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the app activity value. | ||
| * | ||
| * @return The app activity value. | ||
| */ | ||
| public String getAppActivity() { | ||
| return appActivity; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the app activity value. | ||
| * | ||
| * @param appActivity The app activity value. | ||
| */ | ||
| public void setAppActivity(String appActivity) { | ||
|
There was a problem hiding this comment. I'm not sure that it is necessary to keep this method. Maybe it is more senseful to keep only getter for activity and make this field final. @SrinivasanTarget What is your opinion? |
||
| this.appActivity = appActivity; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the app wait package value. | ||
| * | ||
| * @return The app wait package value. | ||
| */ | ||
| public String getAppWaitPackage() { | ||
| return appWaitPackage; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the app wait package value. | ||
| * | ||
| * @param appWaitPackage The app wait package value. | ||
| */ | ||
| public void setAppWaitPackage(String appWaitPackage) { | ||
| this.appWaitPackage = appWaitPackage; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the app wait activity value. | ||
| * | ||
| * @return The app wait activity value. | ||
| */ | ||
| public String getAppWaitActivity() { | ||
| return appWaitActivity; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the app wait activity value. | ||
| * | ||
| * @param appWaitActivity The app wait activity value. | ||
| */ | ||
| public void setAppWaitActivity(String appWaitActivity) { | ||
| this.appWaitActivity = appWaitActivity; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the intent action value. | ||
| * | ||
| * @return The intent action value. | ||
| */ | ||
| public String getIntentAction() { | ||
| return intentAction; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the intent action value. | ||
| * | ||
| * @param intentAction The intent action value. | ||
| */ | ||
| public void setIntentAction(String intentAction) { | ||
| this.intentAction = intentAction; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the intent category value. | ||
| * | ||
| * @return The intent category value. | ||
| */ | ||
| public String getIntentCategory() { | ||
| return intentCategory; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the intent category value. | ||
| * | ||
| * @param intentCategory The intent category value. | ||
| */ | ||
| public void setIntentCategory(String intentCategory) { | ||
| this.intentCategory = intentCategory; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the intent flags value. | ||
| * | ||
| * @return The intent flags value. | ||
| */ | ||
| public String getIntentFlags() { | ||
| return intentFlags; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the intent flags value. | ||
| * | ||
| * @param intentFlags The intent flags value. | ||
| */ | ||
| public void setIntentFlags(String intentFlags) { | ||
| this.intentFlags = intentFlags; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the optional intent arguments value. | ||
| * | ||
| * @return The optional intent arguments value. | ||
| */ | ||
| public String getOptionalIntentArguments() { | ||
| return optionalIntentArguments; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the optional intent arguments value. | ||
| * | ||
| * @param optionalIntentArguments The optional intent arguments value. | ||
| */ | ||
| public void setOptionalIntentArguments(String optionalIntentArguments) { | ||
| this.optionalIntentArguments = optionalIntentArguments; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the stop app value. | ||
| * | ||
| * @return The stop app value. | ||
| */ | ||
| public boolean isStopApp() { | ||
| return stopApp; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the stop app value. | ||
| * | ||
| * @param stopApp The stop app value. | ||
| */ | ||
| public void setStopApp(boolean stopApp) { | ||
| this.stopApp = stopApp; | ||
| } | ||
|
|
||
| @Override public String toString() { | ||
| return "Activity{" + "appPackage='" + appPackage + '\'' + ", appActivity='" + appActivity | ||
| + '\'' + ", appWaitPackage='" + appWaitPackage + '\'' + ", appWaitActivity='" | ||
| + appWaitActivity + '\'' + ", intentAction='" + intentAction + '\'' | ||
| + ", intentCategory='" + intentCategory + '\'' + ", intentFlags='" + intentFlags + '\'' | ||
| + ", optionalIntentArguments='" + optionalIntentArguments + '\'' + ", stopApp=" | ||
| + stopApp + '}'; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,31 @@ | |
| import io.appium.java_client.ExecutesMethod; | ||
|
|
||
| public interface StartsActivity extends ExecutesMethod { | ||
| /** | ||
| * This method should start arbitrary activity during a test. If the activity belongs to | ||
| * another application, that application is started and the activity is opened. | ||
| * <p> | ||
| * Usage: | ||
|
There was a problem hiding this comment. @email2vimalraj I'm against this change as it is. Please mark these methods |
||
| * </p> | ||
| * <pre> | ||
| * {@code | ||
| * Activity activity = new Activity(); | ||
| * activity.setAppPackage("com.foo"); | ||
| * activity.setAppActivity(".bar"); | ||
| * driver.startActivity(activity); | ||
| * } | ||
| * </pre> | ||
| * | ||
| * @param activity The {@link Activity} object | ||
| */ | ||
| default void startActivity(Activity activity) { | ||
| CommandExecutionHelper.execute(this, | ||
| startActivityCommand(activity.getAppPackage(), activity.getAppActivity(), | ||
| activity.getAppWaitPackage(), activity.getAppWaitActivity(), | ||
| activity.getIntentAction(), activity.getIntentCategory(), activity.getIntentFlags(), | ||
| activity.getOptionalIntentArguments(), activity.isStopApp())); | ||
| } | ||
|
|
||
| /** | ||
| * This method should start arbitrary activity during a test. If the activity belongs to | ||
| * another application, that application is started and the activity is opened. | ||
|
|
@@ -32,7 +57,9 @@ public interface StartsActivity extends ExecutesMethod { | |
| * @param appWaitPackage Automation will begin after this package starts. [Optional] | ||
| * @param appWaitActivity Automation will begin after this activity starts. [Optional] | ||
| * @param stopApp If true, target app will be stopped. [Optional] | ||
| * @deprecated Instead use {@link #startActivity(Activity)} | ||
| */ | ||
| @Deprecated | ||
| default void startActivity(String appPackage, String appActivity, String appWaitPackage, | ||
| String appWaitActivity, boolean stopApp) throws IllegalArgumentException { | ||
| this.startActivity(appPackage,appActivity,appWaitPackage, | ||
|
|
@@ -47,7 +74,9 @@ default void startActivity(String appPackage, String appActivity, String appWait | |
| * @param appActivity The activity to start. [Required] | ||
| * @param appWaitPackage Automation will begin after this package starts. [Optional] | ||
| * @param appWaitActivity Automation will begin after this activity starts. [Optional] | ||
| * @deprecated Instead use {@link #startActivity(Activity)} | ||
| */ | ||
| @Deprecated | ||
| default void startActivity(String appPackage, String appActivity, String appWaitPackage, | ||
| String appWaitActivity) throws IllegalArgumentException { | ||
| this.startActivity(appPackage, appActivity, | ||
|
|
@@ -60,7 +89,9 @@ default void startActivity(String appPackage, String appActivity, String appWait | |
| * | ||
| * @param appPackage The package containing the activity. [Required] | ||
| * @param appActivity The activity to start. [Required] | ||
| * @deprecated Instead use {@link #startActivity(Activity)} | ||
| */ | ||
| @Deprecated | ||
| default void startActivity(String appPackage, String appActivity) throws IllegalArgumentException { | ||
| this.startActivity(appPackage, appActivity, null, null, | ||
| null,null,null,null,true); | ||
|
|
@@ -79,7 +110,9 @@ default void startActivity(String appPackage, String appActivity) throws Illegal | |
| * @param intentFlags Flags that will be used to start activity [Optional] | ||
| * @param intentOptionalArgs Additional intent arguments that will be used to | ||
| * start activity [Optional] | ||
| * @deprecated Instead use {@link #startActivity(Activity)} | ||
| */ | ||
| @Deprecated | ||
| default void startActivity(String appPackage, String appActivity, | ||
| String appWaitPackage, String appWaitActivity, | ||
| String intentAction, String intentCategory, | ||
|
|
@@ -104,7 +137,9 @@ default void startActivity(String appPackage, String appActivity, | |
| * @param optionalIntentArguments Additional intent arguments that will be used to | ||
| * start activity [Optional] | ||
| * @param stopApp If true, target app will be stopped. [Optional] | ||
| * @deprecated Instead use {@link #startActivity(Activity)} | ||
| */ | ||
| @Deprecated | ||
| default void startActivity(String appPackage, String appActivity, String appWaitPackage, | ||
| String appWaitActivity, String intentAction, | ||
| String intentCategory, String intentFlags, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@email2vimalraj Can you update here too.
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.
@email2vimalraj ping