Skip to content

test: Plumb the flaky flag from envoy_cc_test to the native.cc_test#10009

Merged
lizan merged 1 commit intoenvoyproxy:masterfrom
yanavlasov:flaky
Feb 19, 2020
Merged

test: Plumb the flaky flag from envoy_cc_test to the native.cc_test#10009
lizan merged 1 commit intoenvoyproxy:masterfrom
yanavlasov:flaky

Conversation

@yanavlasov
Copy link
Contributor

@yanavlasov yanavlasov commented Feb 11, 2020

This flag allows test to temporarily be marked flaky while test flakiness is being diagnosed and fixed, to reduce the likelihood of CI failing.

See https://docs.bazel.build/versions/2.0.0/be/common-definitions.html#common-attributes-tests for more info about the flaky flag.

Risk Level: Low
Testing: Unit Tests
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Yan Avlasov yavlasov@google.com

Signed-off-by: Yan Avlasov <yavlasov@google.com>
Copy link
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Nice.

@zuercher
Copy link
Member

zuercher commented Feb 11, 2020

Assigned @lizan in case there's some reason we haven't done this earlier.

@mattklein123
Copy link
Member

This seems like a good thing to have, though on the flip side it would probably mean we would never fix our flakes so I'm on the fence?

@lizan
Copy link
Member

lizan commented Feb 12, 2020

Do we want to use --runs_per_test_detects_flakes=true and --flaky_test_attempts as well? So it can give FLAKY status on CI.

@yanavlasov
Copy link
Contributor Author

@mattklein123 It maybe a better alternative to disabling the test, which is what happens when the reason for flakiness is not obvious or time consuming to fix. Disabled test could get stuck in limbo juts as the flaky attribute. The only difference is that if someone breaks the test outright it would fail even if marked flaky, but if it is disabled, nobody would know.

@lizan I think --runs_per_test_detects_flakes=true is only useful with the --runs_per_test=nnn flag which we do not use in CI.
I would argue against --flaky_test_attempts because it hides newly flaky tests by making CI pass. It could be an option to control CI cost if a lot of CI failures are due to newly flaky tests, so if there is a way to analyze this, this could be a good case for it.

@lizan
Copy link
Member

lizan commented Feb 13, 2020

Nm, my idea was using both of --runs_per_test_detects_flakes=true and --flaky_test_attempts so we can distinct FAILED and FLAKY, but seems FLAKY will make CI pass.

@lizan
Copy link
Member

lizan commented Feb 19, 2020

Let's merge this first, we can decide whether we want to use this later when we mark tests flaky. I'm trying to use this to make coverage less flaky.

@lizan lizan merged commit 1223a2b into envoyproxy:master Feb 19, 2020
PiotrSikora pushed a commit to PiotrSikora/envoy that referenced this pull request Feb 23, 2020
PiotrSikora added a commit to istio/envoy that referenced this pull request Feb 24, 2020
Backport envoyproxy/envoy-wasm#422 and its prerequisite (envoyproxy#10009).

* Plumb the flaky flag from envoy_cc_test to the native.cc_test (envoyproxy#10009)

Signed-off-by: Yan Avlasov <yavlasov@google.com>

* ci: mark //test/integration:protocol_integration_test as flaky. (envoyproxy#422)

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
duderino pushed a commit to duderino/envoy that referenced this pull request Mar 3, 2020
…o#162)

Backport envoyproxy/envoy-wasm#422 and its prerequisite (envoyproxy#10009).

* Plumb the flaky flag from envoy_cc_test to the native.cc_test (envoyproxy#10009)

Signed-off-by: Yan Avlasov <yavlasov@google.com>

* ci: mark //test/integration:protocol_integration_test as flaky. (envoyproxy#422)

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
istio-testing pushed a commit to istio/envoy that referenced this pull request Mar 4, 2020
…fixes. (#180)

* Revert "fix opencensus tracer (#155)"

This reverts commit 063eeb9.

* Revert "Add x-goog-user-proj header for sts credential (#152)"

This reverts commit 37dbbd4.

* Revert "Update GrpcService to add StsService. (envoyproxy#411)"

This reverts commit ab59731.

* Revert "fix tracer ssl credential (#151)"

This reverts commit 02901d0.

* Revert "remove url validation as it is not implemented"

This reverts commit 3eb2101.

* Revert "Use gRPC Security Token Service (STS) to get call credentials (envoyproxy#9101)"

This reverts commit ec6b907.

* Revert "[release-1.4] Use sts for call credential when STS_PORT is provided in node metadata #144 (#148)"

This reverts commit 7081e43.

* Revert "Upgrade gRPC to 1.25 which has gRPC STS feature (#145)"

This reverts commit 03ecfad.

* ci: mark //test/integration:protocol_integration_test as flaky. (#162)

Backport envoyproxy/envoy-wasm#422 and its prerequisite (envoyproxy#10009).

* Plumb the flaky flag from envoy_cc_test to the native.cc_test (envoyproxy#10009)

Signed-off-by: Yan Avlasov <yavlasov@google.com>

* ci: mark //test/integration:protocol_integration_test as flaky. (envoyproxy#422)

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

* Remove wasm filter  stress  test

Signed-off-by: gargnupur <gargnupur@google.com>

* Remove wasm stress  test framework

Signed-off-by: gargnupur <gargnupur@google.com>

Co-authored-by: Piotr Sikora <piotrsikora@google.com>
Co-authored-by: Nupur Garg <37600866+gargnupur@users.noreply.github.com>
@yanavlasov yanavlasov deleted the flaky branch July 30, 2020 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants