hazelcast / hazelcast-nodejs-client

Hazelcast Node.js Client
https://hazelcast.com/clients/node-js/
Apache License 2.0
150 stars 59 forks source link

Check for the minimum size of the bytes read only once per frame [API-1546] #1411

Closed fatihozer0 closed 1 year ago

fatihozer0 commented 1 year ago

I am trying to add test cases which are added to Java (https://github.com/hazelcast/hazelcast/pull/21502#issue-1253610965) to see if there is a problem and be sure for every commit. But there is a problem which does not allow me to use already implemented methods such as getLength or getTotalLength. Can you check please?

I am aware of the name of the file. I am planning to add these test cases to file named ClientMessageReaderTest.js after the end of implementation.

fatihozer0 commented 1 year ago

I am aware of the code quality problems. All the cases are implemented. It looks like there is not a problem like in Java. I think these should be added to ClientMessageReaderTest.js right?

codecov[bot] commented 1 year ago

Codecov Report

Merging #1411 (df79912) into master (f026c81) will decrease coverage by 0.03%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1411      +/-   ##
==========================================
- Coverage   93.37%   93.34%   -0.04%     
==========================================
  Files         466      466              
  Lines       16623    16623              
  Branches     1351     1351              
==========================================
- Hits        15522    15516       -6     
- Misses        799      806       +7     
+ Partials      302      301       -1     
Impacted Files Coverage Δ
src/nearcache/NearCache.ts 92.24% <0.00%> (-1.56%) :arrow_down:
src/proxy/NearCachedMapProxy.ts 93.05% <0.00%> (-1.39%) :arrow_down:
src/network/ConnectionManager.ts 83.99% <0.00%> (-1.17%) :arrow_down:
src/core/HazelcastError.ts 77.61% <0.00%> (ø)
src/protocol/ErrorFactory.ts 64.66% <0.00%> (ø)
src/invocation/InvocationService.ts 94.87% <0.00%> (ø)
src/util/Util.ts 87.67% <0.00%> (+0.68%) :arrow_up:
...rc/serialization/portable/DefaultPortableReader.ts 85.00% <0.00%> (+1.42%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

srknzl commented 1 year ago

@fatihozer0 Sure. You can add to the ClientMessageReaderTest.

srknzl commented 1 year ago

@fatihozer0 Please add milestone, label etc. to the PR

fatihozer0 commented 1 year ago

All review comments are applied.

srknzl commented 1 year ago

@fatihozer0 All comments are not applied see my responses in unresolved comments