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

POC of using page object pattern #7017

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

niklasberglund
Copy link
Collaborator

@niklasberglund niklasberglund commented Oct 18, 2024

POC of using page object pattern for the login tests. Basically moving UI automator interactions to page classes with abstract functions representing user actions in test cases. Automated tests read like manual test cases and code is reused more.

In this POC PR I have not updated the detekt config but if we go with something like this we probably would like to set up some rules for Page subclasses?


This change is Reviewable

@niklasberglund niklasberglund added the Android Issues related to Android label Oct 18, 2024
@niklasberglund niklasberglund self-assigned this Oct 18, 2024
Copy link

linear bot commented Oct 18, 2024

Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @niklasberglund)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/page/LoginPage.kt line 19 at r1 (raw file):

    fun tapLoginButton(): LoginPage {
        val accountTextField = device.findObjectWithTimeout(By.clazz("android.widget.EditText"))
        val loginButton = accountTextField.parent.findObject(By.clazz(Button::class.java))

I don't really understand this, should this not find all buttons in the login screen? Is it possible to do both type and text?


android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/LoginTest.kt line 26 at r1 (raw file):

        LoginPage(device)
            .enterAccountNumber(validTestAccountNumber)

Copy link
Collaborator Author

@niklasberglund niklasberglund left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/page/LoginPage.kt line 19 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I don't really understand this, should this not find all buttons in the login screen? Is it possible to do both type and text?

For this POC I just moved the existing code to this page function. The identification of this button could definitely be improved, but I kept down the scope of this PR.

Copy link
Contributor

@kl kl left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @niklasberglund and @Pururun)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/page/LoginPage.kt line 19 at r1 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

For this POC I just moved the existing code to this page function. The identification of this button could definitely be improved, but I kept down the scope of this PR.

Could also do this maybe?

Code snippet:

    fun tapLoginButton(): LoginPage {
        val loginButton = device.findObject(By.clazz(Button::class.java).text("Login"))
        loginButton.wait(Until.enabled(true), DEFAULT_TIMEOUT)
        loginButton.click()
        return this
    }

Copy link
Collaborator Author

@niklasberglund niklasberglund left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/page/LoginPage.kt line 19 at r1 (raw file):

Previously, kl (Kalle Lindström) wrote…

Could also do this maybe?

IMHO test tags are more robust because text labels and class types can change, but that's another discussion and refactoring maybe 😊 Anyways if it breaks now it would only need to be updated in one place

Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @niklasberglund)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/page/LoginPage.kt line 19 at r1 (raw file):

For this POC I just moved the existing code to this page function. The identification of this button could definitely be improved, but I kept down the scope of this PR.

Right, than just ignore what I said. :)

@albin-mullvad albin-mullvad added the On hold Means the PR is paused for some reason. No need to review it for now label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to Android On hold Means the PR is paused for some reason. No need to review it for now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants