Skip to content

Added Several Contact Support tests.#15744

Merged
tiagomar merged 12 commits intodevelopfrom
adds-contact-us-UI-test
Jan 3, 2022
Merged

Added Several Contact Support tests.#15744
tiagomar merged 12 commits intodevelopfrom
adds-contact-us-UI-test

Conversation

@pachlava
Copy link
Contributor

@pachlava pachlava commented Dec 22, 2021

Description

Since the ability to contact support is considered one of the most critical, this PR adds several tests:

  • helpCanBeOpenedWhileEnteringPassword: checks that Help button is accessible from the password prompt screen.
  • helpCanBeOpenedWhileEnteringPassword: checks that Help button is accessible from the email prompt screen.
  • sendButtonEnabledWhenTextIsEntered: checks that Send button switches states correctly depending on the entered message text.
  • messageCanBeSent (disabled, only executable with gradle.properties from Mobile Secrets, more here): sends a message and checks that a certain reply is returned.

Testing Instructions

  • The Connected Tests-2 CI job is green, and you can see helpCanBeOpenedWhileEnteringPassword, sendButtonEnabledWhenTextIsEntered , helpCanBeOpenedWhileEnteringEmail tests included.
  • Associated Firebase execution is also green (Firebase console access is needed to check this).
  • Tests run locally (both with default gradle.properties and the ones from Mobile Secrets) on Pixel 2 API 28 (this is the model used on CI).
  • [optional] Enable and run messageCanBeSent locally with gradle.properties from Mobile Secrets.

Regression Notes

  1. Potential unintended areas of impact:
    All changes took place in the package related to automated tests. The app behaviour should not be affected.

  2. What I did to test those areas of impact (or what existing automated tests I relied on):
    I executed all existing automated tests, and the CI is green.

  3. What automated tests I added:
    I added:

  • sendButtonEnabledWhenTextIsEntered
  • messageCanBeSent
  • helpCanBeOpenedWhileEnteringEmail
  • helpCanBeOpenedWhileEnteringPassword

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 22, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@pachlava pachlava marked this pull request as draft December 22, 2021 10:01
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 22, 2021

You can test the changes on this Pull Request by downloading the APKs:

@pachlava pachlava added the UI Tests Anything related to automated UI Tests. label Dec 23, 2021
@pachlava pachlava added this to the Future milestone Dec 23, 2021
Comment on lines +29 to +35
static ViewInteraction textInput = onView(allOf(
isCompletelyDisplayed(),
anyOf(
withId(R.id.message_composer_input_text),
withId(R.id.request_message_field)
)
));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This translates to: find an element that is both completely displayed and has either R.id.message_composer_input_text or R.id.request_message_field ID.

In fact, both of these elements are present when the screen is opened, but only one is visible.

}
}

@Ignore("As long as CI does not use gradle.properties from MobileSecrets")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As can be seen from the number of commits in this PR starting with CI fail fix:..., I spent quite a while figuring out why the tests that involve Contact Support screen failed on CI but never failed the same way locally. At some point (thanks to video recording in Firebase) I saw that the screen on CI:

Screenshot 2021-12-23 at 19 58 29

Looks different from the same screen on my local Emulator:

Screenshot 2021-12-23 at 19 52 52

After blaming the Emulator/Android differences for some time, I realized that CI uses the default gradle.properties, which makes the screen look like differently (and the message cannot be sent, hence the disabled test).

Since I automated this test, I decided to keep it for the case if Mobile Secrets will be used on CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

In case we have this test running in CI are we going to be creating real Zendesk tickets? I'm afraid it can get real noisy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have this suspicion too. This test is disabled for now, so it's not an urgent matter, but I think we can contact the HE team, and ask them if there's a painless way for them to filter out certain messages, so that they wouldn't be bothered with them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it turns out to be the case that we cannot prevent the creation of real tickets, and idea could be to use the Zendesk APIs to find the tickets created by the UI tests and delete them.

Comment on lines +40 to +42
} finally {
new ContactSupportScreen().goBackAndDeleteUnsentMessageIfNeeded();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A kind of local teardown, in order to not slow down helpCanBeOpenedWhileEnteringEmail and helpCanBeOpenedWhileEnteringPassword which do not get to ContactSupport screen, so there's no possibility of unsent message.

Comment on lines +54 to +57
onView(anyOf(
withText(android.R.string.ok),
withId(android.R.id.button1)
))
Copy link
Contributor Author

@pachlava pachlava Dec 23, 2021

Choose a reason for hiding this comment

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

This is the locator for OK button in this dialog:

Screenshot 2021-12-23 at 21 35 35

I used something like onView(withText("OK")) initially, and this worked locally, but never on CI. Thanks to this discussion, I found out that "OK" button might be presented differently by different Android versions, while android.R.id.button1 always stands for the "positive" option in the dialog.

@pachlava pachlava marked this pull request as ready for review December 23, 2021 19:41
@pachlava pachlava changed the title [Draft] Added Several Contact Support tests. Added Several Contact Support tests. Dec 23, 2021
Comment on lines +49 to +52
String automatedReplyText = "Mobile support will respond as soon as possible, "
+ "generally within 48-96 hours. "
+ "Please reply with your site address (URL) "
+ "and any additional details we should know.";
Copy link
Contributor

Choose a reason for hiding this comment

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

I have this test failing since the message have slightly changed due to holidays. We might want to do a partial check as the two last lines remained the same.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. So far, I'm not sure if it makes sense to maintain this test (or even to keep it) - I probably firstly need to ask if Zendesk connection is ever planned on CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have it connected for WPiOS because it was not possible to even check if the screen had been loaded. From what I can recall it is possible but not desirable. Given it is an identified critical flow I'd ask @jkmassel to weigh in. =)

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also use a less sophisticated check and ensure we get any text in R.id.request_system_message_text. 🤷‍♂️

Comment on lines +478 to +491
public static boolean waitForElementToBeDisplayedWithoutFailure(final ViewInteraction element) {
try {
waitForConditionToBeTrueWithoutFailure(new Supplier<Boolean>() {
@Override
public Boolean get() {
return isElementDisplayed(element);
}
});
} catch (Exception e) {
// ignore the failure
}
return isElementDisplayed(element);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I had to do the same thing in my PR. The only difference was that I made waitForElementToBeDisplayedWithoutFailure(final Integer elementID) call waitForElementToBeDisplayedWithoutFailure(final ViewInteraction element) to avoid a bit of code duplication.
Nothing to do here.

Copy link
Contributor

@tiagomar tiagomar left a comment

Choose a reason for hiding this comment

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

CI is green, I ran the tests locally several times and the only issue I found was the Zendesk response message which seems to change seasonally, however this test is disabled so no impact is expected in CI and it will no longer be a problem the next week. That said, I feel comfortable to approve this PR and have this test fixed later on in another PR. :shipit:

Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together!

}
}

@Ignore("As long as CI does not use gradle.properties from MobileSecrets")
Copy link
Contributor

Choose a reason for hiding this comment

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

If it turns out to be the case that we cannot prevent the creation of real tickets, and idea could be to use the Zendesk APIs to find the tickets created by the UI tests and delete them.

Comment on lines +49 to +52
String automatedReplyText = "Mobile support will respond as soon as possible, "
+ "generally within 48-96 hours. "
+ "Please reply with your site address (URL) "
+ "and any additional details we should know.";
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also use a less sophisticated check and ensure we get any text in R.id.request_system_message_text. 🤷‍♂️

@tiagomar tiagomar merged commit bdcad4c into develop Jan 3, 2022
@tiagomar tiagomar deleted the adds-contact-us-UI-test branch January 3, 2022 20:24
@tiagomar
Copy link
Contributor

tiagomar commented Jan 3, 2022

Since @pachlava will be AFK this week and the PR has two approvals already, I'll merge it now.

@pachlava
Copy link
Contributor Author

Thanks for the review @tiagomar (and for the merge while I was AFK) and @mokagio!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

UI Tests Anything related to automated UI Tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants