listener: add socket api in os sys calls for additional tests #3968
Conversation
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
|
@ggreenway added tests as discussed in #3912. PTAL. |
| NiceMock<Api::MockOsSysCalls> os_sys_calls; | ||
| TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls(&os_sys_calls); | ||
|
|
||
| ON_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillByDefault(Return(-1)); |
There was a problem hiding this comment.
I think this should be EXPECT_CALL
There was a problem hiding this comment.
Also, please add an explicit EXPECT_CALL for AF_INET, that returns >= 0.
| EXPECT_CALL(*listener_foo, onDestroy()); | ||
| } | ||
|
|
||
| // Make sure that a listener is created on IPv6 ony setups. |
| NiceMock<Api::MockOsSysCalls> os_sys_calls; | ||
| TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls(&os_sys_calls); | ||
|
|
||
| ON_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillByDefault(Return(-1)); |
There was a problem hiding this comment.
Same comments as previous test
| ListenerHandle* listener_foo = expectListenerCreate(false); | ||
| EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); | ||
|
|
||
| EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); |
There was a problem hiding this comment.
Shouldn't this fail? The address is ipv4, and this test doesn't support ipv4.
There was a problem hiding this comment.
I should have clarified, I am not testing the "actual" listener creation functionality here, it may fail in the actual setup. But my focus was to test convertDestinationIPsMapToTrie function does not throw exceptions and thus cause listener creation failure of there are no filterMatches defined. I will add this commentary on the test.
Probably that should be validated in the ListenerImpl creation it self. So to your other comment about adding ipv6 address tests does not help.
There was a problem hiding this comment.
Ok, I understand better now.
I still think this test case is odd, because this configuration is not supposed to work. I understand that the test still succeeds because it doesn't complete the creation of the listener, but I think it would be better for this one if you change the listener address in the config snippet to a v6 address.
There was a problem hiding this comment.
Ok. Changed, Please check
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama rama.rao@salesforce.com
Description:
Adds additional test cases for #3912
Add socket api in os sys calls
Risk Level: Low
Testing: Automated tests
Docs Changes: N/A
Release Notes: N/A