Skip to content

stats: Add tests for StatsHandler::handlerStats()#16915

Merged
ggreenway merged 4 commits intoenvoyproxy:mainfrom
RyanTheOptimist:stats_handler_test
Jun 21, 2021
Merged

stats: Add tests for StatsHandler::handlerStats()#16915
ggreenway merged 4 commits intoenvoyproxy:mainfrom
RyanTheOptimist:stats_handler_test

Conversation

@RyanTheOptimist
Copy link
Copy Markdown
Contributor

stats: Add tests for StatsHandler::handlerStats()

Also a small Refactor of StatsHandler::handlerStats().

Risk Level: Low
Testing: Unit tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

/assign @jmarantz

Also a small Refactor of StatsHandler::handlerStats().

Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

@jmarantz Hopefully this is pretty simple. I broke this out of #16535

@jmarantz
Copy link
Copy Markdown
Contributor

/wait

Signed-off-by: Ryan Hamilton <rch@google.com>
Copy link
Copy Markdown
Contributor Author

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

Thanks for the review. Sorry for the delay!

Copy link
Copy Markdown
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

Looks great, with one minor nit.

/assign-from @envoyproxy/senior-maintainers
(there's a small prod refactor).

@repokitteh-read-only
Copy link
Copy Markdown

@envoyproxy/senior-maintainers assignee is @alyssawilk

🐱

Caused by: a #16915 (review) was submitted by @jmarantz.

see: more, trace.

@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

Looks great, with one minor nit.

Thanks! What's the nit?

jmarantz
jmarantz previously approved these changes Jun 17, 2021
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

Somehow this nit got lost. re-sending :)

TestUtility::ipTestParamsToString);

TEST_P(AdminStatsTest, HandlerStatsInvalidFormat) {
std::string url = "/stats?format=blergh";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const here & below.

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.

Oh, good point! Done.

Signed-off-by: Ryan Hamilton <rch@google.com>
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

Looks great; over to Greg for senior maintainer stamp.

@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

@ggreenway ping

@ggreenway ggreenway merged commit 888fc9a into envoyproxy:main Jun 21, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Also a small Refactor of StatsHandler::handlerStats().

Signed-off-by: Ryan Hamilton <rch@google.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.

4 participants