Skip to content
This repository was archived by the owner on Apr 16, 2018. It is now read-only.

Conversation

tkhurana96
Copy link
Contributor

No description provided.

@tkhurana96 tkhurana96 self-assigned this Mar 23, 2017
@tkhurana96 tkhurana96 requested a review from agauniyal March 23, 2017 12:28
@codecov
Copy link

codecov bot commented Mar 23, 2017

Codecov Report

Merging #19 into feature/socket will increase coverage by 5.65%.
The diff coverage is 92.11%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           feature/socket      #19      +/-   ##
==================================================
+ Coverage           79.88%   85.53%   +5.65%     
==================================================
  Files                  11       12       +1     
  Lines                 835     1051     +216     
  Branches               82       94      +12     
==================================================
+ Hits                  667      899     +232     
+ Misses                168      152      -16
Impacted Files Coverage Δ
include/socket.hpp 73.6% <ø> (+6.93%) ⬆️
test/socket_send_test.cpp 97.5% <100%> (+1.84%) ⬆️
test/socket_bind_test.cpp 100% <100%> (ø) ⬆️
test/socket_connect_test.cpp 96.07% <100%> (+4.07%) ⬆️
test/socket_constructor_test.cpp 100% <100%> (ø) ⬆️
test/socket_recv_test.cpp 89.79% <86.2%> (-1.51%) ⬇️
test/socket_read_write_test.cpp 93.57% <93.57%> (ø)
test/socket_options_test.cpp 96.07% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c44849...0d62f15. Read the comment docs.

std::string serverPath("/tmp/unixServer");

unixClient.bind([&](AddrUnix &s) {
return methods::construct(s, &clientPath.front());
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 .c_str()?

return methods::construct(s, &clientPath.front());
});

unixClient.connect(&serverPath.front());
Copy link
Member

Choose a reason for hiding this comment

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

.c_str()?

std::string serverPath("/tmp/unixServer");

unixServer.bind([&](AddrUnix &s) {
return methods::construct(s, &serverPath.front());
Copy link
Member

Choose a reason for hiding this comment

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

.c_str()?

std::thread serverThread1(runUnixServer, std::ref(serverUnixTCP),
&unixPathServer.front());
serverThread1.detach();
std::this_thread::sleep_for(2s);
Copy link
Member

Choose a reason for hiding this comment

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

try lowering 2s delay if possible.

EXPECT_ANY_THROW(Socket s(Domain::IPv4, Type::TCP, 41));
EXPECT_ANY_THROW(Socket s(Domain::IPv4, Type::UDP, 6));

EXPECT_NO_THROW(Socket otherS(Socket(Domain::IPv4, Type::TCP)));
Copy link
Member

Choose a reason for hiding this comment

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

What is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

trying the constructor which takes an rvalue

Copy link
Member

Choose a reason for hiding this comment

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

@tkhurana96 try with a vector.emplace() kind of method then, which is more practical example.

void udpIPv6ServerProcessing(Socket &serverSocket)
{
const auto res = serverSocket.read(readTest::msgLen);
EXPECT_EQ(res, readTest::msg);
Copy link
Member

Choose a reason for hiding this comment

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

Why using EXPECT_EQ?

Socket unixClient1(Domain::UNIX, Type::TCP);
EXPECT_EQ(unixClient1.getType(), Type::TCP);
EXPECT_NO_THROW(unixClient1.bind([&](AddrUnix &s) {
return methods::construct(s, &readTest::unixClientPath1.front());
Copy link
Member

Choose a reason for hiding this comment

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

.c.str()?

EXPECT_NO_THROW(unixClient1.bind([&](AddrUnix &s) {
return methods::construct(s, &readTest::unixClientPath1.front());
}));
EXPECT_NO_THROW(unixClient1.connect(&readTest::unixServerPath1.front()));
Copy link
Member

Choose a reason for hiding this comment

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

.c.str()?

Socket unixClient2(Domain::UNIX, Type::UDP);
EXPECT_EQ(unixClient2.getType(), Type::UDP);
EXPECT_NO_THROW(unixClient2.bind([&](AddrUnix &s) {
return methods::construct(s, &readTest::unixClientPath2.front());
Copy link
Member

Choose a reason for hiding this comment

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

.c.str()?

EXPECT_NO_THROW(unixClient2.bind([&](AddrUnix &s) {
return methods::construct(s, &readTest::unixClientPath2.front());
}));
EXPECT_NO_THROW(unixClient2.connect(&readTest::unixServerPath2.front()));
Copy link
Member

Choose a reason for hiding this comment

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

.c.str()?

@agauniyal agauniyal merged commit 720028d into feature/socket Mar 24, 2017
@agauniyal agauniyal deleted the improveCoverage branch March 24, 2017 18:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants