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

Alert API Implementation for iOS #459

Merged
merged 1 commit into from
Aug 19, 2016
Merged

Alert API Implementation for iOS #459

merged 1 commit into from
Aug 19, 2016

Conversation

SrinivasanTarget
Copy link
Member

@SrinivasanTarget SrinivasanTarget commented Aug 16, 2016

Change list

Alert Handling for iOS.Added below methods,

1)getAlertText
2)setAlertText(String value)
3)acceptAlert
4)dismissAlert

API is here: https://github.com/appium/appium-base-driver/blob/master/lib/mjsonwp/routes.js#L403

API's were not implemented for Android Driver on server side.We can port it to android driver on client side post server implementation.

New W3C API's here: https://github.com/appium/appium-base-driver/blob/master/lib/mjsonwp/routes.js#L414 are not been adapted by Appium (Legacy drivers on server) yet.

Types of changes

What types of changes are you proposing/introducing to Java client?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@TikhomirovSergey please review

@TikhomirovSergey
Copy link
Contributor

TikhomirovSergey commented Aug 16, 2016

@SrinivasanTarget
Like this solution but I am not agree with this implementation. It duplicates methods of the org.openqa.selenium.Alert API. Why could we use Selenium API more high? Like the sample below.

public class IOSDriver<T extends WebElement>
    extends AppiumDriver<T>
    implements IOSDeviceActionShortcuts,
        FindsByIosUIAutomation<T> {
...

    @Override public TargetLocator switchTo() {
        return new InnerTargetLocator();
    }

    private class InnerTargetLocator extends RemoteTargetLocator {
        @Override public Alert alert() {
            return new IOSAlert(super.alert());
        }
    }

    class IOSAlert implements Alert {
        private final Alert alert;

        IOSAlert(Alert alert) {
            this.alert = alert;
        }

        @Override
        public void dismiss() {
            alert.dismiss();
        }

        @Override
        public void accept() {
            alert.accept();
        }

        @Override
        public String getText() {
            Response response = execute(DriverCommand.GET_ALERT_TEXT);
            return response.getValue().toString();
        }

        @Override
        public void sendKeys(String keysToSend) {
            execute(DriverCommand.SET_ALERT_VALUE, prepareArguments("value", keysToSend));
        }

        @Override
        public void setCredentials(Credentials credentials) {
            alert.setCredentials(credentials);
        }

        @Override
        public void authenticateUsing(Credentials credentials) {
            alert.authenticateUsing(credentials);
        }
    }
}

@TikhomirovSergey
Copy link
Contributor

TikhomirovSergey commented Aug 18, 2016

There are new checkstyle issues:
image

@SrinivasanTarget Please get it fixed

.IosUIAutomation(".elements().withName(\"show alert\")")).click();
WebDriverWait wating = new WebDriverWait(driver, 10000);
wating.until(alertIsPresent());
assertNotNull(driver.switchTo().alert().getText());
Copy link
Contributor

Choose a reason for hiding this comment

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

@SrinivasanTarget
I would like to advive you to

assertTrue(!StringUtils.isBlank(driver.switchTo().alert().getText()));

What if the empty string would be returned? :)

@SrinivasanTarget
Copy link
Member Author

@TikhomirovSergey Thanks.I have incorporated the changes now and squashed the commits.

Implementation changes

Fixed Checkstyle issues

Fixed Checkstyle issues
@TikhomirovSergey TikhomirovSergey merged commit 1e61110 into appium:master Aug 19, 2016
@SrinivasanTarget SrinivasanTarget deleted the alerthandling branch August 20, 2016 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants