Skip to content

Conversation

@kavyasrinet
Copy link

No description provided.

// Read from channel
int out;
ch->Receive(&out); // should block since channel is empty
current = 10;
Copy link
Collaborator

Choose a reason for hiding this comment

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

10 is a magical number

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the review.
I just realized that we already checked in the test cases : BufferedChannelCloseUnblocksReceiversTest and UnbufferedChannelCloseUnblocksReceiversTest that perform these operations already.
In that case, this PR would be redundant and maybe I can close it.

}

void ReceiveEmptyChannelTest(Channel* ch) {
size_t current = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

5 is a magical number

void ReceiveEmptyChannelTest(Channel* ch) {
size_t current = 5;
std::thread t([&]() {
// Read from channel
Copy link
Collaborator

Choose a reason for hiding this comment

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

unnecessary comment

delete ch;
}

void ReceiveEmptyChannelTest(Channel* ch) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The prefix Test is a convention of unit test names. This function is not a unit test; a better name could be ReceiveFromEmptyChannelShouldBlock.

@wangkuiyi
Copy link
Collaborator

A rewrite is as follows:

void ReceiveFromEmptyChannelShouldBlockAndCloseUnblocksIt(Channel *ch) { bool received = false; std::thread t([&]() { int out; ch->Receive(&out); // should block since channel is empty received = true; }); std::this_thread::sleep_for(std::chrono::milliseconds(100)); // wait 0.1 sec EXPECT_EQ(received, false); // the receiver should be blocked since channel is empty CloseChannel(ch); t.join(); EXPECT_EQ(received, true); // when we close the channel, the receiver should unblock delete ch; } TEST(Channel, ReceiveFromEmptyBufferedChannelShouldBlockAndCloseUnblocksIt) { auto ch = MakeChannel<size_t>(3); // buffered ReceiveFromEmptyChannelShouldBlockAndCloseUnblocksIt(ch); } TEST(Channel, ReceiveFromEmptyUnBufferedChannelShouldBlockAndCloseUnblocksIt) { auto ch = MakeChannel<size_t>(0); // unbuffered ReceiveEmptyChannelTest(ch); }
@kavyasrinet kavyasrinet closed this Feb 6, 2018
@kavyasrinet kavyasrinet deleted the recv_block_empty branch February 8, 2018 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants