cli: fix TestNEP17Balance test #3319
Conversation
AliceInHunterland
requested review from
AnnaShaleva and
roman-khimov
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3319 +/- ##
==========================================
- Coverage 84.96% 84.96% -0.01%
==========================================
Files 328 328
Lines 44789 44789
==========================================
- Hits 38057 38055 -2
Misses 5221 5221
- Partials 1511 1513 +2 ☔ View full report in Codecov by Sentry. |
cli/nep_test/nep17_test.go
Outdated
| getUtilityTokenBalance := func(acc util.Uint160) *big.Int { | ||
| _, err := e.Chain.GetTokenLastUpdated(acc) | ||
| require.NoError(t, err) | ||
| return e.Chain.GetUtilityTokenBalance(acc) | ||
| } |
There was a problem hiding this comment.
How this could prevent the described test failure? Have you checked, is it a race or something else? Why the race is happening (if it's a race)?
There was a problem hiding this comment.
I'm asking because I'd like to make 100% sure that we found and fixed the source of the problem correctly.
There was a problem hiding this comment.
the initial hypothesis was that if on the server side, we do GetTokenLastUpdated(acc), then we need it in test https://github.com/nspcc-dev/neo-go/blob/master/pkg/services/rpcsrv/server.go#L1185
however, with the last changes here, I could reproduce the failure of the test.
AliceInHunterland
force-pushed
the
test-token-balance
branch
from
49a41ff
to
f0dca2f
Compare
| gasState := e.Chain.GetContractState(gasHash) | ||
| require.NotNil(t, gasState) | ||
|
|
||
| lub, err := e.Chain.GetTokenLastUpdated(acc) |
There was a problem hiding this comment.
I don't see how call to last updated height is connected with this race. Could you explain?
There was a problem hiding this comment.
It doesn't...
AliceInHunterland
force-pushed
the
test-token-balance
branch
2 times, most recently
from
03ad7f4
to
5ad949a
Compare
Signed-off-by: Ekaterina Pavlova <[email protected]>
AliceInHunterland
force-pushed
the
test-token-balance
branch
from
5ad949a
to
8fcfb4d
Compare
|
Replaced by #3332. |
As there was a race in gas balance the
e.Chain.GetTokenLastUpdatedneeded before checking the balance for synchronisation rpc request result and chain's value.Close #2960