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

Fix broken e2e tests #5632

Merged
merged 2 commits into from
Jan 5, 2024
Merged

Fix broken e2e tests #5632

merged 2 commits into from
Jan 5, 2024

Conversation

sabercodic
Copy link
Contributor

@sabercodic sabercodic commented Dec 29, 2023

After migration into android compose and lack of IDs some of ui automate tests stops working.
this are the fixes:

  • LoginTest
  • ConnectionTest
  • WebLinkTest

This change is Reviewable

@sabercodic sabercodic added the Android Issues related to Android label Dec 29, 2023
Copy link

linear bot commented Dec 29, 2023

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 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @sabercodic)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/interactor/AppInteractor.kt line 81 at r1 (raw file):

        return device
            .findObjectWithTimeout(
                By.res("location_info_connection_out_test_tag"),

I will create an issue for this, really silly that we can't use the constants here.


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

    @JvmField
    val permissionRule: GrantPermissionRule =
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {

Why was this removed?

Copy link
Contributor Author

@sabercodic sabercodic 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/interactor/AppInteractor.kt line 81 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I will create an issue for this, really silly that we can't use the constants here.

created, please chack it: https://linear.app/mullvad/issue/DROID-610/make-constraints-accessible-for-the-test-modules


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

Previously, Pururun (Jonatan Rhodin) wrote…

Why was this removed?

The reason to add this permission in first place was to cover storage permission in new android versions, on this PR I find that it is just for our own storage access and we just need it for take screen shots and based on documentations App does not need any extra permission for this: https://developer.android.com/training/data-storage/shared/media#storage-permission-not-always-needed

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 @sabercodic)


android/test/common/src/main/kotlin/net/mullvad/mullvadvpn/test/common/interactor/AppInteractor.kt line 81 at r1 (raw file):

Previously, sabercodic wrote…

created, please chack it: https://linear.app/mullvad/issue/DROID-610/make-constraints-accessible-for-the-test-modules

Nice 👍


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

Previously, sabercodic wrote…

The reason to add this permission in first place was to cover storage permission in new android versions, on this PR I find that it is just for our own storage access and we just need it for take screen shots and based on documentations App does not need any extra permission for this: https://developer.android.com/training/data-storage/shared/media#storage-permission-not-always-needed

Ah ok, as long as it works that's fine, but I guess we should not request write and read external storage for newer versions of android if it is not needed?

Copy link
Contributor Author

@sabercodic sabercodic 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 @Pururun)


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

Previously, Pururun (Jonatan Rhodin) wrote…

Ah ok, as long as it works that's fine, but I guess we should not request write and read external storage for newer versions of android if it is not needed?

yes

@sabercodic sabercodic force-pushed the fix-broken-e2e-tests-droid-609 branch from 4e3d301 to 3e7969b Compare January 2, 2024 12:09
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.

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Collaborator

@albin-mullvad albin-mullvad 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 6 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sabercodic)


android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/EndToEndTest.kt line 34 at r2 (raw file):

            )
        } else {
            null

Has this been verified on multiple API levels? Remember that screenshots are only taken if the tests are failing.

Code quote:

null

Copy link
Contributor Author

@sabercodic sabercodic 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 @albin-mullvad)


android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/EndToEndTest.kt line 34 at r2 (raw file):

Previously, albin-mullvad wrote…

Has this been verified on multiple API levels? Remember that screenshots are only taken if the tests are failing.

Yes, It tested on Android 8, 10, 12 and 14

Copy link
Collaborator

@albin-mullvad albin-mullvad left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@albin-mullvad albin-mullvad force-pushed the fix-broken-e2e-tests-droid-609 branch from 3e7969b to d123576 Compare January 5, 2024 09:44
@albin-mullvad albin-mullvad merged commit f37b615 into main Jan 5, 2024
15 checks passed
@albin-mullvad albin-mullvad deleted the fix-broken-e2e-tests-droid-609 branch January 5, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants