test: add more info for TestDisableSchedulingServiceFallback #8946
Conversation
Signed-off-by: bufferflies <[email protected]>
ti-chi-bot
bot
added
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
ti-chi-bot
bot
added
dco-signoff: yes
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8946 +/- ##
==========================================
- Coverage 76.32% 76.27% -0.06%
==========================================
Files 463 463
Lines 70474 70474
==========================================
- Hits 53792 53751 -41
- Misses 13339 13375 +36
- Partials 3343 3348 +5
Flags with carried forward coverage won't be shown. Click here to find out more. |
Signed-off-by: bufferflies <[email protected]>
ti-chi-bot
bot
removed
the
do-not-merge/work-in-progress
| testutil.Eventually(re, func() bool { | ||
| re.NotNil(suite.pdLeader.GetServer()) | ||
| re.NotNil(suite.pdLeader.GetServer().GetRaftCluster()) | ||
| return suite.pdLeader.GetServer().GetRaftCluster().IsSchedulingControllerRunning() |
There was a problem hiding this comment.
how about return suite.pdLeader.GetServer()!=nil && suite.pdLeader.GetServer().GetRaftCluster()!=nil && suite.pdLeader.GetServer().GetRaftCluster().IsSchedulingControllerRunning()
There was a problem hiding this comment.
I just want to see who is nil.
There was a problem hiding this comment.
re.NotNil also will make test unstable
There was a problem hiding this comment.
how about change titile to test: add more info for TestDisableSchedulingServiceFallback?
bufferflies
changed the title
ti-chi-bot
bot
added
needs-1-more-lgtm
|
@bufferflies: You cannot manually add or delete the reviewing state labels, only I and the tursted members have permission to do so. In response to adding label named lgtm. 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 ti-community-infra/tichi repository. |
|
/merge |
|
@bufferflies: We have migrated to builtin 👉 Please use
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 ti-community-infra/tichi repository. |
| @@ -106,7 +106,7 @@ func (suite *serverTestSuite) TestAllocIDAfterLeaderChange() { | |||
| re := suite.Require() | |||
| re.NoError(failpoint.Enable("github.com/tikv/pd/pkg/mcs/scheduling/server/fastUpdateMember", `return(true)`)) | |||
| pd2, err := suite.cluster.Join(suite.ctx) | |||
| re.NoError(err) | |||
| re.NoError(err, "error: %v", err) | |||
There was a problem hiding this comment.
If it fails, it will print the error. So why do you add this?🤔
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lhy1024, okJiang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
What problem does this PR solve?
Issue Number: ref #8926
What has changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
Release note