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

test(android): add instrumentation test github action #4178

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

jasonboukheir
Copy link
Contributor

@jasonboukheir jasonboukheir commented Mar 17, 2024

Fixes #2311

Copy link

vercel bot commented Mar 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
firezone ⬜️ Ignored (Inspect) Visit Preview Apr 24, 2024 5:55pm

Copy link

github-actions bot commented Mar 17, 2024

Terraform Cloud Plan Output

Plan: 15 to add, 14 to change, 15 to destroy.

Terraform Cloud Plan

Copy link

github-actions bot commented Mar 17, 2024

Performance Test Results

TCP

Test Name Received/s Sent/s Retransmits
direct-tcp-client2server 245.3 MiB (+2%) 246.9 MiB (+2%) 469 (+98%)
direct-tcp-server2client 244.1 MiB (+0%) 245.4 MiB (+0%) 194 (-32%)
relayed-tcp-client2server 220.3 MiB (-2%) 221.0 MiB (-2%) 198 (-15%)
relayed-tcp-server2client 235.5 MiB (-1%) 236.7 MiB (-1%) 385 (+2%)

UDP

Test Name Total/s Jitter Lost
direct-udp-client2server 50.0 MiB (+0%) 0.04ms (-87%) 0.00% (NaN%)
direct-udp-server2client 50.0 MiB (-0%) 0.02ms (+126%) 0.00% (NaN%)
relayed-udp-client2server 50.0 MiB (-0%) 0.04ms (-22%) 0.00% (NaN%)
relayed-udp-server2client 50.0 MiB (+0%) 0.02ms (-39%) 0.00% (NaN%)

@jamilbk
Copy link
Member

jamilbk commented Apr 10, 2024

@jasonboukheir should this be wrapped up and reviewed or be closed you think?

@jasonboukheir
Copy link
Contributor Author

Found a nifty utility for e2e ui testing. There's a way to record an Espresso test in Android Studio. It's under Run > Record Espresso Test

@jasonboukheir
Copy link
Contributor Author

@jasonboukheir should this be wrapped up and reviewed or be closed you think?

It should be g2g for review now as long as the smoke tests can pass

Copy link
Member

@jamilbk jamilbk left a comment

Choose a reason for hiding this comment

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

Good foundation. You need to rebase off main and looks like you have a failing test.

.github/workflows/_rust.yml Outdated Show resolved Hide resolved
Comment on lines +79 to +80
ui-test:
name: ui-test-api-${{ matrix.api-level }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling it a ui-test because that has specific meaning in an android context. See: https://developer.android.com/training/testing/instrumented-tests/ui-tests

Other UI tests can base it off of this :)

@jasonboukheir jasonboukheir force-pushed the jasonboukheir/issue/2311 branch 3 times, most recently from 7e37bdf to e6f8edc Compare April 17, 2024 18:27
@jasonboukheir
Copy link
Contributor Author

There are some issues with the synchronization of the tests... Espresso is complicated...

@jasonboukheir
Copy link
Contributor Author

I've gone back and forth on how to handle the delay in the splash screen. Originally, I checked with a timeout for the Settings button to appear in the view. However, this caused an error:

Waited for the root of the view hierarchy to have window focus and not request layout for 10 seconds. If you specified a non default root matcher, it may be picking a root that never takes focus.

Searching that up online reveals that this can occur when there is a popup window in android that doesn't belong to the application. I tried following solutions in the stack exchange, but the tests were still failing.

Trying Option 2

So I tried a different route, to manage the delay by injecting coroutine scopes. I used a custom CoroutineDispatcher that notified Espresso of an idling resource, hopefully when the delay was called. However, I was not able to get that working. I read through the comments in this issue many times:

Kotlin/kotlinx.coroutines#242

However, I was unable to get the delay call to run properly. It would either freeze the test indefinitely or cause the test to never dispatch the part of the function afterwards.

Going back to Option 1

At the end of the comments in Kotlin/kotlinx.coroutines#242, there is a point to avoid using a custom dispatcher with delay calls, since they are usually reserved for things that intentionally do not idle (hence they should not be treated as an idling resource). The suggestion is to wait for the view to show up, similar to how I was doing it originally. I went back to option 1 and was back to the original issue I was facing.

The solution (for now)

I noticed that tests tended to fail based on the state of the android emulator, so I tried removing the AVD cache step in the flow. This caused the test to take longer to run, but it doesn't fail anymore with that obscure error about finding a root view.

@jasonboukheir
Copy link
Contributor Author

Sorry... I spoke too soon... test is still very flaky 🤯

Copy link
Member

@jamilbk jamilbk left a comment

Choose a reason for hiding this comment

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

LGTM, just a question about CI cleanup. Trying to keep the number of CI dependencies low to reduce supply chain surface.

Comment on lines +90 to +98
- name: Free Disk Space (Ubuntu)
uses: jlumbroso/free-disk-space@main
with:
tool-cache: false
android: false
dotnet: true
haskell: true
large-packages: false
swap-storage: true
Copy link
Member

Choose a reason for hiding this comment

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

Is this still needed?

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, I run into issues with space on the runners

@jasonboukheir jasonboukheir added this pull request to the merge queue Apr 30, 2024
Merged via the queue into main with commit 4baf0cb Apr 30, 2024
135 checks passed
@jasonboukheir jasonboukheir deleted the jasonboukheir/issue/2311 branch April 30, 2024 17:21
@jamilbk
Copy link
Member

jamilbk commented May 5, 2024

@jasonboukheir Unfortunately this is still flaky:

https://github.com/firezone/firezone/actions/runs/8955104222/job/24595479753?pr=4887

I'm going to disable this test for now. Could you spend a bit more time making it robust? I disabled the API 26 test because it was timing out, but it looks like the API 29 one randomly times out as well, so we would want to fix that too.

Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android UI tests
2 participants