Skip to content

Fix test quota cache#3794

Merged
JimmyCYJ merged 4 commits intoistio:masterfrom
JimmyCYJ:fix-TestQuotaCache
Feb 27, 2018
Merged

Fix test quota cache#3794
JimmyCYJ merged 4 commits intoistio:masterfrom
JimmyCYJ:fix-TestQuotaCache

Conversation

@JimmyCYJ
Copy link
Copy Markdown
Member

@JimmyCYJ JimmyCYJ commented Feb 27, 2018 •
edited
Loading

Fixes flaky test in mixer/test/client/quota_cache/quota_cache_test.go, #3660

@JimmyCYJ JimmyCYJ requested a review from a team February 27, 2018 00:42
@JimmyCYJ JimmyCYJ requested a review from qiwzhang February 27, 2018 00:42
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 27, 2018 •
edited
Loading

Codecov Report

Merging #3794 into master will increase coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3794   +/-   ##
======================================
+ Coverage      76%     76%   +1%     
======================================
  Files         291     291           
  Lines       26869   26913   +44     
======================================
+ Hits        20379   20433   +54     
+ Misses       5178    5171    -7     
+ Partials     1312    1309    -3
Impacted Files Coverage Δ
mixer/adapter/stackdriver/stackdriver.go 55% <0%> (-15%) ⬇️
mixer/adapter/servicecontrol/handler.go 39% <0%> (-7%) ⬇️
...olarwinds/internal/papertrail/papertrail_logger.go 56% <0%> (-6%) ⬇️
mixer/adapter/prometheus/server.go 91% <0%> (-3%) ⬇️
mixer/adapter/opa/opa.go 79% <0%> (-3%) ⬇️
mixer/adapter/rbac/rbac.go 10% <0%> (-1%) ⬇️
mixer/adapter/fluentd/fluentd.go 76% <0%> (-1%) ⬇️
mixer/adapter/denier/denier.go 93% <0%> (ø) ⬇️
mixer/adapter/memquota/memquota.go 100% <0%> (ø) ⬆️
mixer/adapter/list/ipList.go 100% <0%> (ø) ⬆️
... and 6 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 c24c689...ea85f4a. Read the comment docs.

qiwzhang
qiwzhang reviewed Feb 27, 2018
View reviewed changes
mixer/test/client/quota_cache/quota_cache_test.go Outdated
Copy link
Copy Markdown
Contributor

@qiwzhang qiwzhang Feb 27, 2018

Choose a reason for hiding this comment

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

if not deterministic, one way is
we don't need to verify them.

or it is better to check < 5

@JimmyCYJ
Copy link
Copy Markdown
Member Author

JimmyCYJ commented Feb 27, 2018

I have changed function VerifyStatsGE to VerifyStatsLT, please take a look.

@qiwzhang
Copy link
Copy Markdown
Contributor

qiwzhang commented Feb 27, 2018

/lgtm
/approve

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Feb 27, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiwzhang

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

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Feb 27, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@JimmyCYJ JimmyCYJ merged commit a431605 into istio:master Feb 27, 2018
@JimmyCYJ JimmyCYJ deleted the fix-TestQuotaCache branch February 27, 2018 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@qiwzhang qiwzhang qiwzhang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@qiwzhang qiwzhang

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JimmyCYJ @qiwzhang @istio-merge-robot @googlebot @istio-testing