Skip to content

test: Re-enable guarddog tests which were accidentally disabled due to a parameterized test mishap.#6462

Merged
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
jmarantz:enable-guarddog-tests
Apr 2, 2019
Merged

test: Re-enable guarddog tests which were accidentally disabled due to a parameterized test mishap.#6462
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
jmarantz:enable-guarddog-tests

Conversation

@jmarantz
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz commented Apr 2, 2019

Description: Prior to final review in #6369 I made the test base-class parameterized to run it both with simulated time and real time. I was actually surprised to find that real-time tests worked in tsan 1000x. It was too good to be true. SimulatedTime is very robust but some of the real-time tests don't work.

I found when looking at coverage that some of the guarddog code wasn't being run in tests which surprised me, because I knew we should have coverage.

So this PR parametarizes all the test-classes, not just the base-class, getting the coverage back.
Risk Level: low
Testing: just this test, but with/without tsan and --runs_per_test=1000
Docs Changes: n/a
Release Notes: n/a

…o a parameterized test mishap.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Do we have an issue tracking deflaking these tests? If so can you link to it in comments or in PR?

jmarantz added 2 commits April 2, 2019 14:04
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@mattklein123 mattklein123 self-assigned this Apr 2, 2019
@mattklein123 mattklein123 merged commit 2f097dd into envoyproxy:master Apr 2, 2019
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.

2 participants