Skip to content

Add integration test for dictionary race#7962

Closed
bianpengyuan wants to merge 1 commit intoistio:masterfrom
bianpengyuan:api-dictionary
Closed

Add integration test for dictionary race#7962
bianpengyuan wants to merge 1 commit intoistio:masterfrom
bianpengyuan:api-dictionary

Conversation

@bianpengyuan
Copy link
Copy Markdown
Contributor

@bianpengyuan bianpengyuan commented Aug 16, 2018 •
edited
Loading

Verified that the test will fail if api sha is not updated.

fixes #7934

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Aug 16, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: hklai

If they are not already assigned, you can assign the PR to them by writing /assign @hklai in a comment when ready.

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kyessenov
kyessenov reviewed Aug 16, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

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

Looks like some test that sets checkDict to true is missing.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 16, 2018 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #7962    +/-   ##
=======================================
- Coverage      71%     71%   -<1%     
=======================================
  Files         374     374            
  Lines       32730   32764    +34     
=======================================
- Hits        23064   23000    -64     
- Misses       8657    8762   +105     
+ Partials     1009    1002     -7
Impacted Files Coverage Δ
mixer/pkg/attribute/list.gen.go 100% <ø> (ø) ⬆️
...er/adapter/solarwinds/internal/appoptics/client.go 40% <0%> (-28%) ⬇️
...t/pkg/serviceregistry/external/servicediscovery.go 79% <0%> (-11%) ⬇️
pilot/pkg/serviceregistry/aggregate/controller.go 69% <0%> (-7%) ⬇️
pilot/pkg/serviceregistry/memory/discovery.go 52% <0%> (-4%) ⬇️
galley/pkg/kube/converter/proto.go 86% <0%> (-2%) ⬇️
mixer/adapter/fluentd/fluentd.go 74% <0%> (-1%) ⬇️
mixer/adapter/opa/opa.go 79% <0%> (-1%) ⬇️
...g/serviceregistry/cloudfoundry/servicediscovery.go 92% <0%> (-1%) ⬇️
pilot/pkg/serviceregistry/consul/controller.go 70% <0%> (-1%) ⬇️
... and 22 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 3742c59...dd8c0cf. Read the comment docs.

@bianpengyuan
Copy link
Copy Markdown
Contributor Author

bianpengyuan commented Aug 16, 2018

/hold wait for istio/api#611

@bianpengyuan bianpengyuan added do-not-merge/hold Block automatic merging of a PR. and removed do-not-merge/hold Block automatic merging of a PR. labels Aug 16, 2018
@bianpengyuan
Copy link
Copy Markdown
Contributor Author

bianpengyuan commented Aug 16, 2018

@mandarjog @kyessenov API dep updated. ptal.

kyessenov
kyessenov reviewed Aug 16, 2018
View reviewed changes
mixer/test/client/env/setup.go Outdated
Copy link
Copy Markdown
Contributor

@kyessenov kyessenov Aug 16, 2018

Choose a reason for hiding this comment

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

Can you set it to true in at least one of mixer/test/client tests?

Copy link
Copy Markdown
Contributor Author

@bianpengyuan bianpengyuan Aug 16, 2018

Choose a reason for hiding this comment

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

So sorry! forgot to commit the new test files...

@bianpengyuan
Copy link
Copy Markdown
Contributor Author

bianpengyuan commented Aug 16, 2018

/retest

@bianpengyuan bianpengyuan force-pushed the api-dictionary branch 2 times, most recently from 9795897 to a938984 Compare August 17, 2018 00:50
@bianpengyuan bianpengyuan changed the base branch from master to release-1.0 August 17, 2018 00:52
@bianpengyuan bianpengyuan changed the base branch from release-1.0 to master August 17, 2018 01:29
@bianpengyuan bianpengyuan force-pushed the api-dictionary branch 2 times, most recently from c269153 to 90c7068 Compare August 17, 2018 17:33
@bianpengyuan
Copy link
Copy Markdown
Contributor Author

bianpengyuan commented Aug 17, 2018

/test e2e-bookInfo-envoyv2-v1alpha3

1 similar comment
@bianpengyuan
Copy link
Copy Markdown
Contributor Author

bianpengyuan commented Aug 17, 2018

/test e2e-bookInfo-envoyv2-v1alpha3

@bianpengyuan bianpengyuan mentioned this pull request Aug 17, 2018
Closed
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Aug 20, 2018

@bianpengyuan: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-bookInfoTests-v1alpha3.sh 90c70682538536b85d49a7c0f820117c5dbfe8d0 link /test e2e-bookInfo-envoyv2-v1alpha3
prow/istio-presubmit.sh dd8c0cf link /test istio-presubmit
prow/istio-unit-tests.sh dd8c0cf link /test istio-unit-tests
Details

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. I understand the commands that are listed here.

@bianpengyuan bianpengyuan mentioned this pull request Aug 20, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyessenov kyessenov kyessenov left review comments

@mandarjog mandarjog Awaiting requested review from mandarjog

@qiwzhang qiwzhang Awaiting requested review from qiwzhang

@andraxylia andraxylia Awaiting requested review from andraxylia

@wattli wattli Awaiting requested review from wattli

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Mixer Report: attribute index not defined error

4 participants

@bianpengyuan @istio-testing @kyessenov @googlebot