client: Improve testing and increase coverage

[cthulhu-rider]

• inline conn code from api-go
• refactor client to use grpc.Conn directly
• prepare the ground for testing operations over gRPC
• add a typical test of one RPC
• do the rest similarly

[codecov]

Codecov Report

Attention: Patch coverage is 68.72428% with 152 lines in your changes missing coverage. Please review.

Project coverage is 58.80%. Comparing base (2da6182) to head (195a8f2).
Report is 17 commits behind head on master.

Files with missing lines	Patch %	Lines
client/container.go	57.14%	28 Missing and 14 partials ⚠️
client/object_get.go	67.56%	18 Missing and 6 partials ⚠️
client/object_put.go	60.00%	16 Missing and 2 partials ⚠️
client/object_replicate.go	57.14%	8 Missing and 4 partials ⚠️
client/reputation.go	57.14%	8 Missing and 4 partials ⚠️
client/netmap.go	75.00%	6 Missing and 4 partials ⚠️
client/client.go	78.94%	5 Missing and 3 partials ⚠️
client/session.go	57.14%	4 Missing and 2 partials ⚠️
client/common.go	81.48%	5 Missing ⚠️
client/object_delete.go	66.66%	2 Missing and 2 partials ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
+ Coverage   57.47%   58.80%   +1.32%     
==========================================
  Files         164      165       +1     
  Lines       22503    22751     +248     
==========================================
+ Hits        12934    13378     +444     
+ Misses       9175     8935     -240     
- Partials      394      438      +44     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cthulhu-rider]

oops, streamTimeout regressed, will return

[roman-khimov]

A number of good fixes.

Regarding embedded gRPC --- it makes testing harder at this level, previously gRPC was layered out and could be tested on its own. I fear we'd get a complete node in tests soon. At the same time we have to handle gRPC somewhere anyway and going through the whole cycle can be beneficial. Take httptest for example, it's an HTTP server and many APIs are tested against it (like NeoGo's JSON-RPC). gRPC is not much different here.

[roman-khimov]

Likely you can just defer c.sendStatistic(...)() and handle statisticCallback and startTime internally.

[cthulhu-rider]

if if is inevitable, then it is better to make a conditional defer imo

[carpawell]

but why? you turned out the func that is commonly used and made more lines, no? i think it was planned as defer c.sendStatistic(...)() but smth went wrong, why make things harder?

[roman-khimov]

Not doing a defer is obviously more efficient, but for these (network) operations the difference is so negligible that I'd rather opt for simpler code.

[cthulhu-rider]

i've done this not just for negligible optimization but for more clear code. Previously, i saw unconditional deferred stats, making me think that some stats are always tracked while sendStatistics encapsulates the logic that there can be no stats at all! Also, seeing defer func() { c.method()() }() blows my mind, i definitely cannot agree this is a simpler code

finally, if no one has any categorical objections, i'd leave the fixed version

[carpawell]

any categorical objections

mine is many repetitive lines for every method. also, you still use defer so why not, at least i would try to inline time calculation: time.Since(time.Now())

seeing defer func() { c.method()() }() blows my mind

agree but once you get it you just see in every method, it is our general approach

[cthulhu-rider]

at least i would try to inline time calculation: time.Since(time.Now())

hm, how would it be?

[carpawell]

i meant start time can be an arg of the deferred func, provided wrong example

[cthulhu-rider]

done

[cthulhu-rider]

it makes testing harder at this level

yeah, tests will definitely become harder todo, but the closer test code is to the real execution, the better it works in error and regression control. I want to be able to track problems at all transmission levels

i also believe that after all tests will be ready, future support wont be so hard todo

I fear we'd get a complete node in tests soon.

to some extent it will be so, and I think it is inevitable. For example, u wanna test that the sent request is correctly signed. To do this, u still have to intercept and check it. Then u wanna check that the client will return a certain result to the user for a certain server response. So, the abstraction interface should work with both requests and responses, as it does in the proposed version. That means that regardless of whether gRPC layer is abstracted or not, test implementations will be almost the same. That's why i included it into the test framework

u can also see some gRPC code we used has already become deprecated. To make sure our future upgrade passed purely and compatible, we should not abstract this level