status-im / status-go

The Status module that consumes go-ethereum
https://status.im
Mozilla Public License 2.0
727 stars 245 forks source link

[Deliverable] Improve tests in status-go related to chat functionalities #5738

Open plopezlpz opened 3 weeks ago

plopezlpz commented 3 weeks ago

As stated here we want to:

  1. Review and improve coverage of chat functionalities tests:

    1. List functionalities that we want to tests
    2. find out their coverage, do we have tests for them? are the current tests flacky?
    3. add tests if missing, correct if flacky
  2. Review and remove the dependency of tests on external systems if appropriate (e.g.: Waku fleet). In order not to depend on a waku fleet for the tests we have the following options:

    1. use docker with a disposable waku fleet and make tests use that, for that we might need to make the bootstrap nodes non-hardcoded
    2. use a live waku fleet intended for testing only (e.g.: staging.fleet adding a shard reserved for tests only, this will mean that multiple run of tests will utilize the same fleet and shard; also we have to take into account possible hardcoded shards in status-go).
    3. maybe have a clear separation between unit and integration tests, some tests should mock Waku, some should utilze either a test fleet or docker.
plopezlpz commented 3 weeks ago

@ilmotta @iurimatias @igor-sirotin @fbarbu15 Let me know your opinion

igor-sirotin commented 3 weeks ago

cc @anastasiyaig @antdanchenko

igor-sirotin commented 3 weeks ago

Overall sounds good 👍 I also thought we could keep tests with real fleets (if they're needed), but only run them nightly. All PR-level tests should not depend on real network conditions.

e.g.: staging.fleet adding a shard reserved for tests only

I wouldn't rely on using particular shard, this kinda modifies the test vs. prod behaviour.

ilmotta commented 3 weeks ago

maybe have a clear separation between unit and integration tests, some tests should mock Waku, some should utilze either a test fleet or docker.

I tend to prefer option 2 -> III @plopezlpz, i.e. we ideally balance out the test pyramid and have a deterministic interface to Waku for the majority of tests, but keep some key tests hitting the real thing.

I also thought we could keep tests with real fleets (if they're needed), but only run them nightly. All PR-level tests should not depend on real network conditions.

Maybe tests relying on the network could run in PRs, but they wouldn't work as stop gates? This could give devs insights into potential problems before merging. As long as these test errors are kind of human-readable, there are not too many of these tests, and they are relatively reliable.

fbarbu15 commented 1 week ago
  1. List functionalities that we want to tests

We probably need Status Dev or QA support here

2. find out their coverage, do we have tests for them? are the current tests flacky?

VAC QA team can help here, we did similar stuff from nwaku, go-waku etc

3. add tests if missing, correct if flacky

Same here

  1. use docker with a disposable waku fleet and make tests use that, for that we might need to make the bootstrap nodes non-hardcoded

We use a similar strategy in the the waku interop tests where we create a small network of nodes using docker containers before each test. That way we have good control on what each test will run on, able to easily run tests locally or on CI, extract node logs etc

2. use a live waku fleet intended for testing only (e.g.: staging.fleet adding a shard reserved for tests only, this will mean that multiple run of tests will utilize the same fleet and shard; also we have to take into account possible hardcoded shards in status-go).

We don't have much experience setting up and hosting such fleet. Maybe VAC DST team or some devops can better assist here

3. maybe have a clear separation between unit and integration tests, some tests should mock Waku, some should utilze either a test fleet or docker.

Agree, I think unit tests should mock it, while integration should use docker

anastasiyaig commented 1 week ago

@antdanchenko @glitchminer ^^