Skip to content

test: fix nit after #2591#2601

Merged
htuch merged 2 commits intoenvoyproxy:masterfrom
dio:nits-after-2591
Feb 15, 2018
Merged

test: fix nit after #2591#2601
htuch merged 2 commits intoenvoyproxy:masterfrom
dio:nits-after-2591

Conversation

@dio
Copy link
Member

@dio dio commented Feb 14, 2018

test: fix nit after #2591

Description:
Fix some nit for PR #2591.

Risk Level: Low

Testing:
unit

Docs Changes:
N/A

Release Notes:
N/A

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.

Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Why not this line as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, OK. BTW @htuch do you think it is worth adding more tests as V2 YAML in this test file?

Copy link
Member

Choose a reason for hiding this comment

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

Generally, moving as much as possible to v2 YAML will be a good thing. I think the best model to do it is organically, e.g. as we go and modify or add new tests, but any cleanup is always appreciated.

@dnoe dnoe self-assigned this Feb 14, 2018
@dnoe
Copy link
Contributor

dnoe commented Feb 14, 2018

Looks like the TSAN failure is a flake, I am investigating that. When you address @htuch's comment and push a new commit it will re-run the test.

htuch
htuch previously approved these changes Feb 15, 2018
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.

Ta.

@htuch
Copy link
Member

htuch commented Feb 15, 2018

Please merge master to pick up #2613, this should fix CI TSAN.

dio added 2 commits February 15, 2018 10:59
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
@dio
Copy link
Member Author

dio commented Feb 15, 2018

It seems fine now 🙌

@htuch htuch merged commit f79a62b into envoyproxy:master Feb 15, 2018
Shikugawa pushed a commit to Shikugawa/envoy that referenced this pull request Mar 28, 2020
* add configurable metric export interval

* format

* update env variable
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Add a setPerTryIdleTimeoutSeconds method to EngineBuilder.
Add tests for setPerTryIdleTimeoutSeconds and setStreamIdleTimeoutSeconds.
Minor cleanup of envoy_config_test.cc.

Part of #2498

Risk Level: Low
Testing: Added unit tests
Docs Changes: N/A
Release Notes: Updated version_history.txt

Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Add a setPerTryIdleTimeoutSeconds method to EngineBuilder.
Add tests for setPerTryIdleTimeoutSeconds and setStreamIdleTimeoutSeconds.
Minor cleanup of envoy_config_test.cc.

Part of #2498

Risk Level: Low
Testing: Added unit tests
Docs Changes: N/A
Release Notes: Updated version_history.txt

Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: JP Simard <jp@jpsim.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.

3 participants