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

tests: avoid some test branches not being reached #8087

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

lhy1024
Copy link
Contributor

@lhy1024 lhy1024 commented Apr 17, 2024

What problem does this PR solve?

Issue Number: Close #8073

What is changed and how does it work?

schedulePeerPr always be 1.0, which makes some test cannot reach transfer-leader branch

Check List

Tests

  • Unit test

Release note

None.

Signed-off-by: lhy1024 <admin@liudos.us>
Copy link
Contributor

ti-chi-bot bot commented Apr 17, 2024

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • HuSharp

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 ti-chi-bot bot requested review from JmPotato and rleungx April 17, 2024 14:02
@ti-chi-bot ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 17, 2024
@@ -527,25 +534,12 @@ func checkHotWriteRegionScheduleByteRateOnly(re *require.Assertions, enablePlace
clearPendingInfluence(hb.(*hotScheduler))
tc.SetHotRegionScheduleLimit(int(opt.GetHotRegionScheduleLimit()))

for i := 0; i < 20; i++ {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part is duplicated with L500-L522. But this branch, which tests the transfer leader, is not reached, so it is not updated before.

Comment on lines +714 to +715
pdServerCfg := tc.GetPDServerConfig()
pdServerCfg.FlowRoundByDigit = 8
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add config to avoid test branches not being reached

re.Equal("move-hot-write-leader", op.Desc())
operatorutil.CheckTransferLearner(re, op, operator.OpHotRegion, 8, 10)
re.Equal("move-hot-write-peer", op.Desc())
operatorutil.CheckTransferLearner(re, op, operator.OpHotRegion, 8, 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it may be scheduled to store 10 or store 11

Signed-off-by: lhy1024 <admin@liudos.us>
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

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

Project coverage is 77.38%. Comparing base (dd7f2a7) to head (4d442df).

Current head 4d442df differs from pull request most recent head 51f79e9

Please upload reports for the commit 51f79e9 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8087      +/-   ##
==========================================
- Coverage   77.40%   77.38%   -0.03%     
==========================================
  Files         471      471              
  Lines       61456    61355     -101     
==========================================
- Hits        47569    47477      -92     
+ Misses      10323    10315       -8     
+ Partials     3564     3563       -1     
Flag Coverage Δ
unittests 77.38% <62.50%> (-0.03%) ⬇️

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

@ti-chi-bot ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: transfer hot region test not run
2 participants