Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify that service cluster is set in case LS tracing is enabled #383

Merged
merged 7 commits into from
Feb 6, 2017

Conversation

RomanDzhabarov
Copy link
Member

No description provided.

@mattklein123
Copy link
Member

test please. you might move the tracing configuration load to a static utility function that is easier to test.

@moderation
Copy link
Contributor

@RomanDzhabarov thanks for following up on my testing issue with this PR.

@RomanDzhabarov
Copy link
Member Author

Sounds good, will add tests in a bit today.


NiceMock<Server::MockInstance> server;
std::string cluster_not_set = "";
EXPECT_CALL(server.local_info_, clusterName()).WillOnce(ReturnRef(cluster_not_set));
Copy link
Member

Choose a reason for hiding this comment

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

Just modify server.local_info_.cluster_name_ directly. You don't need a new variable or the EXPECT_CALL.

@@ -119,5 +120,38 @@ TEST(ConfigurationImplTest, BadListenerConfig) {
EXPECT_THROW(config.initialize(*loader), Json::Exception);
}

TEST(ConfigurationImplTest, NotSetServiceClusterWhenLSTracing) {
Copy link
Member

Choose a reason for hiding this comment

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

ServiceClusterNotSetWhenLSTracing

@mattklein123 mattklein123 reopened this Feb 6, 2017
@RomanDzhabarov RomanDzhabarov merged commit 18f8a33 into master Feb 6, 2017
@mattklein123 mattklein123 deleted the verify_service_cluster branch February 12, 2017 22:52
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
* Add tcp filter

* collect some data.

* Add WriteFilter.

* integrate with mixer_control.

* Add integration test.

* use NamedNetworkFilerConfigFactory
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