FlowCrypt / flowcrypt-android

FlowCrypt Android App
https://flowcrypt.com
Other
91 stars 11 forks source link

more reliable tests #1105

Closed tomholub closed 3 years ago

tomholub commented 3 years ago

Looks good, adding a code clarity request.

Not sure about tests, our Android tests can be quite brittle when run in CI, so it's not clear if it passes or not sometimes until Den runs them locally. Hopefully over time it can be improved.

Yes, Android tests constantly fail here or there. Can we do something to improve that? Last time I've increased time limit and they've passed - maybe another increase needed?

_Originally posted by @IvanPizhenko in https://github.com/FlowCrypt/flowcrypt-android/pull/1103#discussion_r598249644_


Current approach to making sure that tests run on CI has been to disable certain tests that don't run well on CI and only run them locally. But that is not ideal. Since Den is able to run all of the tests locally reliably, there must be something we can do to make them run reliably in CI too.

Ideally we would run all available tests on CI and they would run with reasonable reliability.

tomholub commented 3 years ago

I'm testing #1104 as a quick attempt at some improvement, but it will need a more holistic approach.

DenBond7 commented 3 years ago

@tomholub @IvanPizhenko Unfortunately, while we use Node( actually NDK) I can't guarantee the reliability of the tests on CI. For now, I see a few issues that make our tests worse:

  1. Using Node(actually NDK). Sometimes in logs, I see crashes that relate to NDK. And most of them relate to Android Emulator.
  2. I can't use Android Test Orchestrator while we use Node.
  3. Very often I see something like this one java.util.concurrent.ExecutionException: java.lang.RuntimeException: java.lang.RuntimeException: com.android.ddmlib.AdbCommandRejectedException: device offline in the Semaphore CI logs. And other lines of the logs like this one Tests on ci-test-pixel-x86-api30(AVD) - 11 failed: Test run failed to complete. Expected 19 tests, received 10 says that an emulator was stopped. Maybe such an issue relates to 1.
  4. This one https://github.com/FlowCrypt/flowcrypt-android/pull/1104#issuecomment-803846601 can relate to UI bugs on an emulator. For example, Android OS can show ANR dialog or something else. It should be investigated.

But that is not ideal

I totally agree. But for now, it's the best choice. Currently, I dropped all tests that sometimes fail on CI. So on CI we have tests that should work. Just need to restart them time-to-time due to 1-4 cases

tomholub commented 3 years ago

I see. That means once we drop node, test reliability should improve? I'll close this issue then.

DenBond7 commented 3 years ago

I see. That means once we drop node, test reliability should improve? I'll close this issue then.

yes, you are right