search

status-im / status-go

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

Remove API/wrappers from test coverage #5466

Open igor-sirotin opened 5 days ago

igor-sirotin commented 5 days ago

Problem

Example PRs:

Implementation

There're 4 ways to approach this.

1. Write the tests

You'd never thought of this, but it is a way as well 😄 For example: https://github.com/status-im/status-go/blob/e0c762c0d4d4e07c4356a31a859ae3ef2d9b4bc0/services/wallet/api_test.go#L13-L23

2. Remove according files from the report

Just the same as we do with ./cmd/*: https://github.com/status-im/status-go/blob/e2846f3936d7ffab833f1eef68b500d6224055cc/_assets/scripts/run_unit_tests.sh#L141-L142

3. Re-arrange the files, so that api/wrappers packages doesn't contain any tests

The packages that don't have any tests at all, don't affect the coverage report. If we wouldn't have those files:

4. Don't write wrappers

I'm not sure if this is possible. But I think gethWakuV2Wrapper could be replaced with an interface, e.g. Or some other solution that would make it possible not to write such api/wrappers at all.

Notes

Not all api.go functions are pure wrappers. It's important to ensure that (if we exclude such files from coverage report) we simplify such functions to wrap-only funcionality: https://github.com/status-im/status-go/blob/e0673ad1ffec65e3bd96a46b5054fc2d36071cc4/services/ext/api.go#L1435-L1450

Samyoul commented 4 days ago

I think we should implement solution 2 now and then work towards implementing solution 3.

igor-sirotin commented 3 days ago

Keeping those, as they don't have tests:

At this file services/communitytokens/api.go I though that I'm not sure anymore if this is a good idea to do. Half of the functions are pure wrappers, half of them have logic that should be covered with tests.

I'll think about it, but no I think that we shouldn't exclude such files. In most cases they shouldn't be a problem because a usual wrapper is 1 line of code, while the wrapped function is more and it should be covered with tests. That should give enough coverage to pass 50%.