coverage: revert workarounds that are no longer neccessary #10837
Conversation
Fixes envoyproxy#10108 Signed-off-by: Greg Greenway <ggreenway@apple.com>
|
@ggreenway coverage test is not always to fail, it depends how gtest shard tests into 50 runs, i.e. adding/removing test change the sharding. At some point it will break. It is always better to shuffle test order to detect flakiness / unintended dependencies between tests. |
|
Hmm. What I was trying to fight is inconsistent coverage results between runs. Are we better off closing this so the tests don’t get stuck in a broken state? |
|
Can we potentially keep the shuffle but remove the flaky tag so it runs once? We have fixed a bunch of things and also have stack traces now so it should be easier to understand what happens if it fails during the flake run? |
|
Sounds reasonable to me. @lizan wdyt? |
|
SGTM |
Signed-off-by: Greg Greenway <ggreenway@apple.com>
|
The irony: coverage failed. Sigh. Maybe we shouldn't shuffle and just deal with whatever issues show up? |
|
I'm confused. For that coverage run, there were no failures, but it says coverage was only 96.5%. The SHUFFLE shouldn't affect the overall coverage, just whether it passes/fails, right? |
I'm actually not sure this is the case. I could see the order mattering in terms of the calculations, especially wrt to death tests and other strange things. |
|
I'm fine with removing the SHUFFLE and seeing how it goes from there. |
This reverts commit 315f274. Signed-off-by: Greg Greenway <ggreenway@apple.com>
b7ec98f to
ca5002b
Compare
…y#10837) Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: pengg <pengg@google.com>
Description: Turn off 'Flaky' for coverage tests.
Risk Level: Low
Signed-off-by: Greg Greenway ggreenway@apple.com