tidb-dashboard icon indicating copy to clipboard operation
tidb-dashboard copied to clipboard

fix(keyviz): find nearest id if not found

Open shhdgit opened this issue 3 years ago • 5 comments

shhdgit avatar May 19 '22 01:05 shhdgit

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment. After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review. Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot avatar May 19 '22 01:05 ti-chi-bot

Codecov Report

Attention: Patch coverage is 64.63415% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 31.94%. Comparing base (f3519f0) to head (cb475d4). Report is 208 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1268      +/-   ##
==========================================
+ Coverage   31.77%   31.94%   +0.17%     
==========================================
  Files         310      310              
  Lines       17642    17712      +70     
  Branches      806      806              
==========================================
+ Hits         5605     5658      +53     
- Misses      11805    11821      +16     
- Partials      232      233       +1     
Flag Coverage Δ
be_integration_test_nightly 9.28% <0.00%> (-0.06%) :arrow_down:
be_unit_test 24.50% <64.63%> (+0.38%) :arrow_up:
e2e_test 69.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Sentry.

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

codecov-commenter avatar May 19 '22 01:05 codecov-commenter

Please fix the lint.

Also I believe you need more tests to cover the "findOne" function. For such complicated logic function, maybe better to ensure 100% branch coverage.

breezewish avatar May 19 '22 12:05 breezewish

How about using sort.Search to discover a lower bound, and then check whether it meets upper bound? In this way you don't need to manually write the binary search or lower_bound algorithm.

Here is an example of using sort.Search to implement lower_bound: https://go-review.googlesource.com/c/go/+/29333/3/src/sort/example_search_test.go#45

breezewish avatar May 23 '22 05:05 breezewish

@shhdgit: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ti-chi-bot avatar Nov 08 '22 01:11 ti-chi-bot