Skip to content

network: update network test-coverage expectations and add one simple coverage improvement#14547

Merged
mattklein123 merged 1 commit intoenvoyproxy:masterfrom
jmarantz:network-coverage
Jan 3, 2021
Merged

network: update network test-coverage expectations and add one simple coverage improvement#14547
mattklein123 merged 1 commit intoenvoyproxy:masterfrom
jmarantz:network-coverage

Conversation

@jmarantz
Copy link
Contributor

@jmarantz jmarantz commented Jan 1, 2021

Commit Message: It looks like #14472 brought us to the edge of coverage passing for source/common/network, so this reverts the change to that expectation. I also found one easy savings of a few lines of missing coverage by removing some redundant if statements.
Additional Description:
Risk Level: low
Testing: test/common/network/...
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz requested a review from alyssawilk January 1, 2021 16:16
@mattklein123 mattklein123 self-assigned this Jan 1, 2021
"source/common/init:96.4"
"source/common/json:90.6"
"source/common/network:95.3"
"source/common/network:95.1"
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to drop this lower?

/wait-any

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I meant to revert the effects of #14472 on this particular directory. I think Alyssa's earlier "ratcheting" PR was a bit aggressive and there's something flaky in this test-tree that affects coverage.

I don't think my opportunistic simplification in transport_socket_options_impl.cc will reliably get us closer to 95.3. I think the main issue in this directory is a bunch of constexpr conditionals that were added recently.

Copy link
Member

Choose a reason for hiding this comment

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

OK got it, thanks.

@mattklein123 mattklein123 merged commit 606fc2f into envoyproxy:master Jan 3, 2021
@jmarantz jmarantz deleted the network-coverage branch January 3, 2021 01:28
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