Disable the flaky test and make instrumentation-tests build required

[dzinad]

There is one flaky instrumentation test, described here: https://github.com/mapbox/mapbox-navigation-android/issues/5931. Because of it the instrumentation-tests CI build is optional. And some real failure can be easily missed since the build is red quite often. In order to avoid merging PRs that break instrumentation tests we could do the following:

1. Temporarily disable RouteRefreshTest#routeRefreshesWorksAfterSettingsNewRoutes;
2. Make the instrumentation-tests build required;
3. Enable RouteRefreshTest#routeRefreshesWorksAfterSettingsNewRoutes after fix (should probably add this point to the mentioned ticket).

[VysotskiVadim]

I'm not sure that RouteRefreshTest#routeRefreshesWorksAfterSettingsNewRoutes the only one that makes our test suite flaky. But the solution could work I think

[zugaldia]

Enable RouteRefreshTest#routeRefreshesWorksAfterSettingsNewRoutes after fix (should probably add this point to the mentioned ticket).

Do we understand the root cause for the flakiness of this test? I'd like to understand it better before moving forward with disabling it - which is otherwise a reasonable approach to make instrumentation tests mandatory.

[dzinad]

As I understand it, it fails by timeout. Maybe some emulators are lagging but we'd need to investigate further.

[VysotskiVadim]

Do we understand the root cause for the flakiness of this test?

As far as I understand this ticket, @dzinad please correct me if you meant something different, we need to apply a different approach for handling of flaky instrumentation test. We used to make the instrumentation tests run not required if the tests fail so often that it's hard to merge a PR. New suggestion is to always keep instrumentation tests run mandatary, but to disable individual tests if they fail, to make sure we run and verify at least subset of tests.

[dzinad]

As far as I understand this ticket, @dzinad please correct me if you meant something different, we need to apply a different approach for handling of flaky instrumentation test. We used to make the instrumentation tests run not required if the tests fail so often that it's hard to merge a PR. New suggestion is to always keep instrumentation tests run mandatary, but to disable individual tests if they fail, to make sure we run and verify at least subset of tests.

Exactly. If we want, we can create a separate build for flaky tests which will be optional. But the main point is that the majority of the tests are mandatory for every PR.

[dzinad]

Seems like failures are fixed. However, in several PRs I get "inconclusive" results. Does anybody know what to do with it? @mapbox/navigation-android Example: https://circleci.com/gh/mapbox/mapbox-navigation-android/94624?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

[LukasPaczos]

"inconclusive" looks like a Firebase infra issue. Documentation says:

Inconclusive: Test results were inconclusive, possibly due to a Test Lab error.

I think we can ignore the inconclusive results (or re-run a job if needed). If this becomes much more common, we can get in touch with the Firebase support team to get more information.

[dzinad]

I think we can ignore the inconclusive results (or re-run a job if needed).

But this way we won't be able to make the build required. Because the build itself fails.

[LukasPaczos]

What's the frequency of these inconsistent results that you noticed?

In case of an occurrence, we'll have to re-run the test. If this happens often enough to cause significant pain, let's get in touch with support, maybe there's some configuration parameter that we can set or specific devices we can choose to mitigate the issue.

cc @MrSwimmer for visibility

[dzinad]

I noticed it in 2 of 5 of my PRs. But the most recent ones. Maybe it's a new issue. Let's observe for some time.