Skip to content

Add tests to cover HTTP health checker callbacks#1699

Merged
mattklein123 merged 5 commits intoenvoyproxy:masterfrom
akonradi:http-health-checker-callback-coverage
Sep 20, 2017
Merged

Add tests to cover HTTP health checker callbacks#1699
mattklein123 merged 5 commits intoenvoyproxy:masterfrom
akonradi:http-health-checker-callback-coverage

Conversation

@akonradi
Copy link
Copy Markdown
Contributor

The methods under test are no-ops, but are not covered by any existing tests. Add some simple sanity checks to make sure that the callbacks don't blow anything up if high water marks are reached.

Ensure that HttpHealthCheckerImpl::HttpActiveHealthCheckSession's
Http::StreamCallbacks implementations are sane. Though the called
functions are still no-ops, they are now no-ops that are covered by
tests.

Signed-off-by: Alex Konradi <akonradi@google.com>
Ensure that the no-op high-water-mark callback implementations for
HttpHealthCheckerImpl::HttpActiveHealthCheckSession::ConnectionCallbackImpl
are sane. They are still no-ops, but now they're covered by tests.

Signed-off-by: Alex Konradi <akonradi@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.

Thank you! Small nit.

EXPECT_CALL(*test_sessions_[0]->interval_timer_, enableTimer(_));
EXPECT_CALL(*test_sessions_[0]->timeout_timer_, disableTimer());

for (auto* callback : test_sessions_[0]->request_encoder_.stream_.callbacks_) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I might add runHighWatermarkCallbacks() and runLowWatermarkCallbacks() here: https://github.com/envoyproxy/envoy/blob/master/test/mocks/http/mocks.h#L153. We may want to use this elsewhere in the future and we can put the loop inside the mock.

Signed-off-by: Alex Konradi <akonradi@google.com>
EXPECT_CALL(*test_sessions_[0]->interval_timer_, enableTimer(_));
EXPECT_CALL(*test_sessions_[0]->timeout_timer_, disableTimer());

for (auto* callback : test_sessions_[0]->client_connection_->callbacks_) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry can you do same thing here also? Thanks. (There are already event helpers IIRC).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, forgot about those ones.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found helpers in the ConnectionImpl class but not in the mock class

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

https://github.com/envoyproxy/envoy/pull/1699/files#diff-e96d2141e2131be66d0db09f98bd4a16R32. Can you move there as it applies to both types of connections?

Signed-off-by: Alex Konradi <akonradi@google.com>
Signed-off-by: Alex Konradi <akonradi@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.

Thank you!

@mattklein123 mattklein123 merged commit ce233ba into envoyproxy:master Sep 20, 2017
@akonradi akonradi deleted the http-health-checker-callback-coverage branch September 20, 2017 21:21
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Description: Updates Android filter interfaces to support receiving StreamIntel on callbacks. Underlying data is not yet populated.
Risk Level: Low
Testing: Local & CI
Docs Changes: Pending

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Description: Updates Android filter interfaces to support receiving StreamIntel on callbacks. Underlying data is not yet populated.
Risk Level: Low
Testing: Local & CI
Docs Changes: Pending

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
mathetake added a commit that referenced this pull request Mar 3, 2026
**Description**

This refactors tests/extproc integration tests:
1. Renamed as tests/data-plane to be usable for Dynamic Module too #1564
2. Extract tests/extproc/mcp into tests/data-plane-mcp to have a
separate concern in each integration tests

**Related Issues/PRs (if applicable)**

Preparation for #90

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
mathetake added a commit that referenced this pull request Mar 3, 2026
)

**Description**

This is a draft introduced in #1699

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
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