Run Status-Mobile Integration tests on PR CI

[J-Son89]

To help avoid breaking changes being added to Status-Go it would be helpful to have the Status Mobile integration tests being run with the new build of Status Go.

Status Mobile is currently working to increase code coverage on rpc endpoints used in the mobile project to act as a contract test with Status-Go.

Notes:
PR from Status Mobile to add a separate CI job to run integration tests in isolation.
status-im/status-mobile#18304

Acceptance Criteria:

• Add CI job to trigger integration tests from Status Mobile using pr build of Status Go

[siddarthkay]

Lets also think about when we want to run this job, perhaps only when a PR has been reviewed etc?
cc @jakubgs @yakimant
Also having a new stage added to CI we also would want to make that stage mandatory so that it is enforceable else PRs could be merged without that stage passing.

[ilmotta]

@siddarthkay, @J-Son89, I'm uncertain about making mobile integration tests a mandatory requirement for merging in status-go.

Recently, we experienced sporadic failures in some integration tests, and these issues were not directly linked to bugs in any specific open PR in status-go. Blocking status-go PRs in such cases would have been detrimental. I believe this kind of problem will reappear.

Regarding treating our integration tests as contract tests, I find it somewhat confusing. While our integration tests invoke RPC endpoints under the hood, our re-frame events go beyond orchestrating RPC calls. Failures in our integration tests may result from bugs in manipulating the app db, improper use of subscriptions, outdated integration tests (as happened with old wallet tests), etc. Such failures do not necessarily indicate a bug in the corresponding status-go PR, so I am hesitant to make our integration tests a blocking factor. Perhaps initiating an automatic "strong signal" (a visible message/comment) in the status-go PR could be a more appropriate initial step.

If we genuinely intend to implement contract tests, it would be beneficial to conduct a more thorough assessment of how consumer/provider-driven contracts could be written, and it is crucial to have the desktop team playing along with us because they are the ones churning status-go the most. There are various approaches to implementing contract tests, but I would refrain from considering our integration tests as a form of contract test.

Interestingly, we currently only have three integration tests: one for adding a contact, one to test app initialization, and another for creating an account. These tests are not well understood and maintained by us mobile developers, which reinforces my belief that we should not yet block status-go PRs.

I'm not suggesting our integration tests are useless. Quite the contrary, they can certainly help identify some bugs in status-go.

cc'ing @Samyoul and @cammellos who probably thought about this before.
cc'ing @yakimant too

[J-Son89]

Sure @ilmotta, happy to discuss and have the details ironed out first.
I wasn't aware of the flaky tests but we should look to remove/improve these.
Seems like we need to better define how we write these integration tests.
If we go for a more contract test approach we might have to rewrite
Some of the current tests to just directly interact with the rpc requests.

Currently there is low coverage for these tests but something we have to start investing more time into.
The wallet team has scoped out covering all rpc end points in integration tests in February and from there on we'll make it part of the acceptance criteria to add an integration test when using a new rpc endpoint (or a different use of an endpoint)

[ilmotta]

Sounds great @J-Son89, but just to expand on your comments a little bit.

I wasn't aware of the flaky tests but we should look to remove/improve these.

I believe the integration tests were okay, but they were failing due to the usage of goroutines in a non-ideal way in status-go. I don't know the specifics. So the tests captured a problem (randomly) but they weren't related to any open PR, so that's the perspective I tried to bring, that they do detect bugs, but not necessarily in a way that should block status-go PRs.

If we go for a more contract test approach we might have to rewrite Some of the current tests to just directly interact with the rpc requests.

I agree. The surface for failure in our integration tests is considerably higher than the scope of the public API exposed by status-go. Behind the scenes our integration tests are even tied to particular UI concerns, like navigation.

I remember old discussions with Erik about having a more explicit layer in status-mobile dealing with RPC endpoints. Maybe then that layer could serve as a sort of contract test. But I know that's easy to say and hard to implement in practice. I've tried once to refactor event handlers and have a separate layer just for RPC, isolating certain domain concerns, it didn't work very well, but it would be good to get back to this.

The wallet team has scoped out covering all rpc end points in integration tests in February

Excellent 👏🏼 I would like to open a PR soon making integration tests more enjoyable to read & write. I just want to get some things done for the account selection crew first.