Skip to content

stats: fix unstable test#10953

Merged
alivxxx merged 2 commits intopingcap:masterfrom
alivxxx:stats-test
Jun 27, 2019
Merged

stats: fix unstable test#10953
alivxxx merged 2 commits intopingcap:masterfrom
alivxxx:stats-test

Conversation

@alivxxx
Copy link
Copy Markdown
Contributor

@alivxxx alivxxx commented Jun 26, 2019

What problem does this PR solve?

FAIL: dump_test.go:26: testStatsSuite.TestConversion
dump_test.go:49:
    assertTableEqual(c, loadTbl, tbl)
handle_test.go:100:
    c.Assert(a.Count, Equals, b.Count)
... obtained int64 = 9
... expected int64 = 3

What is changed and how it works?

Since #10771, there will be a background goroutine to Update even if stats-lease is 0, yet some tests are still under the assumption of no automatic updates. In TestConversion, the explicit Update and background Update may conflict each other, since their read and cache updates are two separate operations, so there may be the chances that the newer updates are overwritten by the older updates.

This PR fixes it by disabling the background goroutine by set the stats-lease to negative.

Also, there are still chances that the wrong result is caused by the failure of some stats operations, but there are no error checks for them, so this PR also checks the error for them.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • None

Related changes

  • None

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 26, 2019

Codecov Report

Merging #10953 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #10953   +/-   ##
===========================================
  Coverage   80.9319%   80.9319%           
===========================================
  Files           418        418           
  Lines         89275      89275           
===========================================
  Hits          72252      72252           
  Misses        11789      11789           
  Partials       5234       5234

@coocood
Copy link
Copy Markdown
Member

coocood commented Jun 27, 2019

LGTM

@alivxxx alivxxx added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 27, 2019
Copy link
Copy Markdown
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@alivxxx alivxxx added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 27, 2019
@alivxxx
Copy link
Copy Markdown
Contributor Author

alivxxx commented Jun 27, 2019

/run-all-tests

@alivxxx alivxxx merged commit 0d563f1 into pingcap:master Jun 27, 2019
@alivxxx alivxxx deleted the stats-test branch June 27, 2019 05:51
sre-bot pushed a commit that referenced this pull request Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/test status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants