Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage flakiness on source/server #15239

Closed
mathetake opened this issue Mar 1, 2021 · 9 comments
Closed

coverage flakiness on source/server #15239

mathetake opened this issue Mar 1, 2021 · 9 comments
Labels
area/test flakes stale stalebot believes this issue/PR has not been touched recently tech debt

Comments

@mathetake
Copy link
Member

mathetake commented Mar 1, 2021

The coverage on source/server recently has been flaky between 94.4 and 95.1 on unrelated PRs, for example #15210 and #15117.

What I have observed:

  • This is somehow deterministic, i.e. on some commits, retesting would not work to go green.
  • The root cause sits in source/server/connection_handler_impl.cc
    • by digging into the report, you can see that these lines would not be covered sometimes.

Temporarily the coverage threshold is loosen in #15238 to stop this being a blocker on unrelated PRs. We should not tighten again the corresponding threshold until we resolve this flakiness.

@alyssawilk
Copy link
Contributor

cc @asraa @antoniovicente did we think this had something to do with ENVOY_BUG?

@asraa
Copy link
Contributor

asraa commented Mar 1, 2021

Hmmm that was fixed by https://github.com/envoyproxy/envoy/pull/15104/files, ENVOY_BUGs act as they do in opt mode while running coverage (non-fatal). If I peek at coverage reports, I maybe would have seen a decrement of line number counts after an ENVOY_BUG I know was tested, but I don't see that anymore. (Then again, I don't necessarily trust line numbers on coverage reports)

Was this the same issue you were seeing then?

@alyssawilk
Copy link
Contributor

Ah curses, the ENVOY_BUG issue came up at the last envoy call as a potential cause of the coverage flakiness. If it's not that (given it's fixed) we'll have to do some digging :-(

@alyssawilk alyssawilk added area/test flakes tech debt and removed triage Issue requires triage labels Mar 1, 2021
@alyssawilk
Copy link
Contributor

Also @mathetake thanks so much for not just the quick fix but finding the flaky area.
@ggreenway would you have cycles to take a look and add some deterministic testing for that area?

@lambdai
Copy link
Contributor

lambdai commented Mar 1, 2021

I am not fully sure ENVOY_BUG is the root cause. One of the KP of conn handler is that there are too many internal classes. It is hard to write the test case to cover these lines.

This is my attempt toward deterministic #15127

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 31, 2021
@mathetake
Copy link
Member Author

not stale

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Apr 1, 2021
@github-actions
Copy link

github-actions bot commented May 1, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label May 1, 2021
@github-actions
Copy link

github-actions bot commented May 8, 2021

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test flakes stale stalebot believes this issue/PR has not been touched recently tech debt
Projects
None yet
Development

No branches or pull requests

4 participants