Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[to #464] add unit tests for exported API of RawKVClient #471

Merged
merged 8 commits into from
Mar 24, 2022

Conversation

iosmanthus
Copy link
Member

@iosmanthus iosmanthus commented Jan 4, 2022

Signed-off-by: iosmanthus [email protected]

What problem does this PR solve?

Issue Number: to #464

Problem Description:

Some of the exported API of RawKVClient is not covered by unit tests and this blocks the compatibility tests. This pull request add more tests to cover these APIs including:

  • batchDelete
  • scan0
  • TiKVIterator
  • ingest

Check List for Tests

This PR has been tested by at least one of the following methods:

  • Unit test
  • Integration test

Signed-off-by: iosmanthus <[email protected]>
Signed-off-by: iosmanthus <[email protected]>
@iosmanthus iosmanthus changed the title [to #464] add unit tests for exported API of RawKVClient [to #464] [WIP] add unit tests for exported API of RawKVClient Jan 4, 2022
Signed-off-by: iosmanthus <[email protected]>
Signed-off-by: iosmanthus <[email protected]>
@iosmanthus iosmanthus changed the title [to #464] [WIP] add unit tests for exported API of RawKVClient [to #464] add unit tests for exported API of RawKVClient Jan 4, 2022
zz-jason
zz-jason previously approved these changes Jan 6, 2022
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iosmanthus you can solve the issues in another pull request

@iosmanthus
Copy link
Member Author

@marsishandsome PTAL

@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #471 (3b56877) into master (92fea32) will increase coverage by 0.35%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #471      +/-   ##
============================================
+ Coverage     32.59%   32.94%   +0.35%     
- Complexity     1324     1329       +5     
============================================
  Files           270      270              
  Lines         17054    17054              
  Branches       1950     1950              
============================================
+ Hits           5559     5619      +60     
+ Misses        10889    10829      -60     
  Partials        606      606              
Impacted Files Coverage Δ
...ty/handler/codec/http2/Http2ConnectionHandler.java 48.65% <0.00%> (-5.14%) ⬇️
...rc/main/java/io/grpc/netty/NettyClientHandler.java 56.89% <0.00%> (-1.94%) ⬇️
src/main/java/io/grpc/internal/ClientCallImpl.java 56.34% <0.00%> (-0.74%) ⬇️
src/main/java/org/tikv/common/PDClient.java 59.27% <0.00%> (-0.49%) ⬇️
src/main/java/org/tikv/common/TiSession.java 70.67% <0.00%> (-0.49%) ⬇️
.../codec/http2/DefaultHttp2RemoteFlowController.java 59.31% <0.00%> (-0.39%) ⬇️
...va/org/tikv/common/region/StoreHealthyChecker.java 75.00% <0.00%> (+1.31%) ⬆️
.../main/java/org/tikv/common/region/RegionCache.java 50.92% <0.00%> (+1.85%) ⬆️
...java/org/tikv/common/region/RegionStoreClient.java 50.61% <0.00%> (+2.76%) ⬆️
src/main/java/org/tikv/raw/RawKVClient.java 73.42% <0.00%> (+11.59%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92fea32...3b56877. Read the comment docs.

marsishandsome
marsishandsome previously approved these changes Mar 23, 2022
Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ti-srebot
ti-srebot previously approved these changes Mar 23, 2022
@zz-jason
Copy link
Member

@iosmanthus please resolve conflicts.

@iosmanthus
Copy link
Member Author

@zz-jason @marsishandsome PTAL

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason enabled auto-merge (squash) March 23, 2022 06:45
@zz-jason zz-jason merged commit 36d0109 into tikv:master Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants