Skip to content

Commit 25196e5

Browse files
blucabluca
authored
Merge pull request #201 from sigiesec/add-poller-tests
Problem: insufficient tests for poller_t, bad usability since caller of add must store function object
2 parents 1975818 + 5b1ab44 commit 25196e5

File tree

2 files changed

+102
-16
lines changed

2 files changed

+102
-16
lines changed

‎tests/poller.cpp

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,62 @@
88
TEST(poller, create_destroy)
99
{
1010
zmq::poller_t poller;
11+
ASSERT_EQ(0u, poller.size ());
1112
}
1213

13-
TEST(poller, move_contruct)
14+
static_assert(!std::is_copy_constructible<zmq::poller_t>::value, "poller_t should not be copy-constructible");
15+
static_assert(!std::is_copy_assignable<zmq::poller_t>::value, "poller_t should not be copy-assignable");
16+
17+
TEST(poller, move_construct_empty)
1418
{
15-
zmq::poller_t poller;
16-
zmq::poller_t poller_move {std::move (poller)};
19+
std::unique_ptr<zmq::poller_t> a{new zmq::poller_t};
20+
zmq::poller_t b = std::move(*a);
21+
22+
ASSERT_EQ(0u, a->size ());
23+
a.reset ();
24+
ASSERT_EQ(0u, b.size ());
1725
}
1826

19-
TEST(poller, move_assign)
27+
TEST(poller, move_assign_empty)
2028
{
21-
zmq::poller_t poller;
22-
zmq::poller_t poller_assign;
23-
poller_assign = std::move (poller);
29+
std::unique_ptr<zmq::poller_t> a{new zmq::poller_t};
30+
zmq::poller_t b;
31+
32+
b = std::move(*a);
33+
34+
ASSERT_EQ(0u, a->size ());
35+
a.reset ();
36+
ASSERT_EQ(0u, b.size ());
37+
}
38+
39+
TEST(poller, move_construct_non_empty)
40+
{
41+
zmq::context_t context;
42+
zmq::socket_t socket{context, zmq::socket_type::router};
43+
44+
std::unique_ptr<zmq::poller_t> a{new zmq::poller_t};
45+
a->add(socket, ZMQ_POLLIN, [](short) {});
46+
zmq::poller_t b = std::move(*a);
47+
48+
ASSERT_EQ(0u, a->size ());
49+
a.reset ();
50+
ASSERT_EQ(1u, b.size ());
51+
}
52+
53+
TEST(poller, move_assign_non_empty)
54+
{
55+
zmq::context_t context;
56+
zmq::socket_t socket{context, zmq::socket_type::router};
57+
58+
std::unique_ptr<zmq::poller_t> a{new zmq::poller_t};
59+
a->add(socket, ZMQ_POLLIN, [](short) {});
60+
zmq::poller_t b;
61+
62+
b = std::move(*a);
63+
64+
ASSERT_EQ(0u, a->size ());
65+
a.reset ();
66+
ASSERT_EQ(1u, b.size ());
2467
}
2568

2669
TEST(poller, add_handler)
@@ -49,12 +92,14 @@ TEST(poller, add_handler_twice_throws)
4992
zmq::poller_t poller;
5093
zmq::poller_t::handler_t handler;
5194
poller.add(socket, ZMQ_POLLIN, handler);
95+
/// \todo the actual error code should be checked
5296
ASSERT_THROW(poller.add(socket, ZMQ_POLLIN, handler), zmq::error_t);
5397
}
5498

5599
TEST(poller, wait_with_no_handlers_throws)
56100
{
57101
zmq::poller_t poller;
102+
/// \todo the actual error code should be checked
58103
ASSERT_THROW(poller.wait(std::chrono::milliseconds{10}), zmq::error_t);
59104
}
60105

@@ -63,16 +108,26 @@ TEST(poller, remove_unregistered_throws)
63108
zmq::context_t context;
64109
zmq::socket_t socket{context, zmq::socket_type::router};
65110
zmq::poller_t poller;
111+
/// \todo the actual error code should be checked
66112
ASSERT_THROW(poller.remove(socket), zmq::error_t);
67113
}
68114

69-
TEST(poller, remove_registered)
115+
/// \todo this should lead to an exception instead
116+
TEST(poller, remove_registered_empty)
70117
{
71118
zmq::context_t context;
72119
zmq::socket_t socket{context, zmq::socket_type::router};
73120
zmq::poller_t poller;
74-
zmq::poller_t::handler_t handler;
75-
poller.add(socket, ZMQ_POLLIN, handler);
121+
poller.add(socket, ZMQ_POLLIN, zmq::poller_t::handler_t{});
122+
ASSERT_NO_THROW(poller.remove(socket));
123+
}
124+
125+
TEST(poller, remove_registered_non_empty)
126+
{
127+
zmq::context_t context;
128+
zmq::socket_t socket{context, zmq::socket_type::router};
129+
zmq::poller_t poller;
130+
poller.add(socket, ZMQ_POLLIN, [](short) {});
76131
ASSERT_NO_THROW(poller.remove(socket));
77132
}
78133

@@ -85,7 +140,7 @@ class loopback_ip4_binder
85140
std::string endpoint() { return endpoint_; }
86141
private:
87142
// Helper function used in constructor
88-
// as Gtest allows only void returning functions
143+
// as Gtest allows ASSERT_* only in void returning functions
89144
// and constructor/destructor are not.
90145
void bind(zmq::socket_t &socket)
91146
{
@@ -112,7 +167,7 @@ TEST(poller, poll_basic)
112167
zmq::socket_t sink{context, zmq::socket_type::pull};
113168
ASSERT_NO_THROW(sink.connect(endpoint));
114169

115-
std::string message = "H";
170+
const std::string message = "H";
116171

117172
// TODO: send overload for string would be handy.
118173
ASSERT_NO_THROW(vent.send(std::begin(message), std::end(message)));
@@ -128,10 +183,11 @@ TEST(poller, poll_basic)
128183
ASSERT_TRUE(message_received);
129184
}
130185

186+
/// \todo this contains multiple test cases that should be split up
131187
TEST(poller, client_server)
132188
{
133189
zmq::context_t context;
134-
std::string send_msg = "Hi";
190+
const std::string send_msg = "Hi";
135191

136192
// Setup server
137193
zmq::socket_t server{context, zmq::socket_type::router};

‎zmq.hpp

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@
2626
#ifndef __ZMQ_HPP_INCLUDED__
2727
#define __ZMQ_HPP_INCLUDED__
2828

29+
#if (__cplusplus >= 201402L)
30+
#define ZMQ_DEPRECATED(msg) [[deprecated(msg)]]
31+
#elif defined(_MSC_VER)
32+
#define ZMQ_DEPRECATED(msg) __declspec(deprecated(msg))
33+
#elif defined(__GNUC__)
34+
#define ZMQ_DEPRECATED(msg) __attribute__((deprecated(msg)))
35+
#endif
36+
2937
#if (__cplusplus >= 201103L)
3038
#define ZMQ_CPP11
3139
#define ZMQ_NOTHROW noexcept
@@ -1022,9 +1030,21 @@ namespace zmq
10221030

10231031
using handler_t = std::function<void(short)>;
10241032

1025-
void add (zmq::socket_t &socket, short events, handler_t &handler)
1033+
ZMQ_DEPRECATED("from 4.3.0, use overload accepting handler_t instead")
1034+
void add (zmq::socket_t &socket, short events, std::function<void(void)> &handler)
1035+
{
1036+
add (socket, events, [&handler](short) { handler(); });
1037+
}
1038+
1039+
void add (zmq::socket_t &socket, short events, handler_t handler)
10261040
{
1027-
if (0 == zmq_poller_add (poller_ptr, socket.ptr, handler ? &handler : NULL, events)) {
1041+
handler_t *handler_ptr = nullptr;
1042+
/// \todo is it sensible to allow handler to be empty? doesn't this lead to an error when the event is signalled? (perhaps it should already lead to an error in zmq_poller_add then)
1043+
if (handler) {
1044+
auto emplace_res = handlers.emplace(&socket, std::move(handler));
1045+
handler_ptr = &emplace_res.first->second;
1046+
}
1047+
if (0 == zmq_poller_add (poller_ptr, socket.ptr, handler_ptr, events)) {
10281048
poller_events.emplace_back (zmq_poller_event_t ());
10291049
return;
10301050
}
@@ -1034,7 +1054,11 @@ namespace zmq
10341054
void remove (zmq::socket_t &socket)
10351055
{
10361056
if (0 == zmq_poller_remove (poller_ptr, socket.ptr)) {
1037-
poller_events.pop_back ();
1057+
auto it = handlers.find (&socket);
1058+
if (it != handlers.end ()) { /// \todo this may only be false if handler was empty on add
1059+
handlers.erase (it);
1060+
}
1061+
poller_events.pop_back ();
10381062
return;
10391063
}
10401064
throw error_t ();
@@ -1060,10 +1084,16 @@ namespace zmq
10601084

10611085
throw error_t ();
10621086
}
1087+
1088+
size_t size ()
1089+
{
1090+
return poller_events.size();
1091+
}
10631092

10641093
private:
10651094
void *poller_ptr;
10661095
std::vector<zmq_poller_event_t> poller_events;
1096+
std::map<socket_t*, handler_t> handlers;
10671097
}; // class poller_t
10681098
#endif // defined(ZMQ_BUILD_DRAFT_API) && defined(ZMQ_CPP11) && defined(ZMQ_HAVE_POLLER)
10691099

0 commit comments

Comments
 (0)