Skip to content

[ci] fix per-file coverage#13030

Merged
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
asraa:per-file-coverge
Sep 10, 2020
Merged

[ci] fix per-file coverage#13030
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
asraa:per-file-coverge

Conversation

@asraa
Copy link
Contributor

@asraa asraa commented Sep 9, 2020

Commit Message: Fix per file extension coverage in CI
Additional Description: Filed issues for new low coverage extensions
Risk Level: Low
Testing: currently testing with docker bazel.coverage after fixing known low coverage, but i admit it takes > 1 hour to run locally. so i'm keeping that running

Filed issues for new watchdog, ocsp oath2 extensions
Self assigned HCM config coverage issue

dynamic_forward_proxy's regressed but the uncovered lines are trivial (https://storage.googleapis.com/envoy-postsubmit/master/coverage/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc.gcov.html)
all others extensions have existing issues

Signed-off-by: Asra Ali asraa@google.com

Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
htuch
htuch previously approved these changes Sep 9, 2020
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Contributor Author

asraa commented Sep 10, 2020

Had to reduce watchdog coverage in the interim, fixed now.

Signed-off-by: Asra Ali <asraa@google.com>
htuch
htuch previously approved these changes Sep 10, 2020
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Contributor Author

asraa commented Sep 10, 2020

I'm really sorry about this but source/common/common regressed by .1 in the last commit is as well. Current post-submit works with the script now, I'll ping when I have all green checks.

@alyssawilk
Copy link
Contributor

Yeah, this was annoying to get in last time too - I think it took me several tries and I snuck it in off hours.
Might be worth asking if Harvey or Matt would be Ok force merging just to avoid coverage races on your side.
Not much we can do about them in the other direction :-/

@mattklein123
Copy link
Member

I'm happy to force merge but then I think main will be broken, but then we can do a fast follow. Up to you.

@alyssawilk
Copy link
Contributor

If she runs off the current coverage.dat it'll be as up to date as it's likely to get. Either way any PR which has passed presubmit but merges without a refresh is going to risk breaking things for a day or so :-/

@asraa
Copy link
Contributor Author

asraa commented Sep 10, 2020

If this try doesn't work, I'm happy to sneak this in after work hours tonight and ping maintainer chat to see if someone's online to approve.

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