Skip to content

Commit 307691f

Browse files
TrentHoulistoncursoragent
TrentHouliston
and
cursoragent
committed
Extract shared queue BDD helpers to cut SonarCloud duplication.
MPSCQueue and TaskQueue tests shared six near-identical scenario bodies; move them into queue_bdd_helpers.hpp so new-code duplication drops below 3%. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 4fb157a commit 307691f

3 files changed

Lines changed: 240 additions & 263 deletions

File tree

‎tests/test_util/queue_bdd_helpers.hpp‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
/*
2+
* MIT License
3+
*
4+
* Copyright (c) 2024 NUClear Contributors
5+
*
6+
* This file is part of the NUClear codebase.
7+
* See https://github.com/Fastcode/NUClear for further info.
8+
*
9+
* Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
10+
* documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
11+
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
12+
* permit persons to whom the Software is furnished to do so, subject to the following conditions:
13+
*
14+
* The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
15+
* Software.
16+
*
17+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
18+
* WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
19+
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
20+
* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
*/
22+
23+
#ifndef TEST_UTIL_QUEUE_BDD_HELPERS_HPP
24+
#define TEST_UTIL_QUEUE_BDD_HELPERS_HPP
25+
26+
#include <atomic>
27+
#include <catch2/catch_test_macros.hpp>
28+
#include <memory>
29+
#include <thread>
30+
#include <type_traits>
31+
#include <utility>
32+
33+
#include "test_util/queue_live_tracker.hpp"
34+
35+
namespace test_util {
36+
namespace queue_bdd {
37+
38+
namespace detail {
39+
40+
template <typename Queue, typename = void>
41+
struct has_empty : std::false_type {};
42+
43+
template <typename Queue>
44+
struct has_empty<Queue, decltype(void(std::declval<const Queue&>().empty()))> : std::true_type {};
45+
46+
template <typename Queue>
47+
void assert_queue_reports_empty(Queue& queue, std::true_type /*has_empty*/) {
48+
CHECK(queue.empty());
49+
}
50+
51+
template <typename Queue>
52+
void assert_queue_reports_empty(Queue& queue, std::false_type /*has_empty*/) {
53+
int discard = 0;
54+
CHECK_FALSE(queue.try_dequeue(discard));
55+
}
56+
57+
template <typename Queue>
58+
void assert_queue_reports_empty(Queue& queue) {
59+
assert_queue_reports_empty(queue, has_empty<Queue>{});
60+
}
61+
62+
} // namespace detail
63+
64+
/// Queue destroyed with items still enqueued runs remaining element destructors.
65+
template <typename Queue>
66+
void destructor_runs_remaining_destructors_scenario() {
67+
GIVEN("A queue filled across several blocks then only partially drained") {
68+
queue_live_tracker_count().store(0, std::memory_order_relaxed);
69+
70+
WHEN("The queue is destroyed with items still enqueued") {
71+
{
72+
Queue queue;
73+
for (int i = 0; i < 200; ++i) {
74+
queue.enqueue(QueueLiveTracker(i));
75+
}
76+
/*drain a few*/ {
77+
QueueLiveTracker sink(-1);
78+
for (int i = 0; i < 10; ++i) {
79+
REQUIRE(queue.try_dequeue(sink));
80+
}
81+
}
82+
83+
// 190 elements remain live inside the queue's blocks.
84+
CHECK(queue_live_tracker_count().load(std::memory_order_relaxed) == 190);
85+
}
86+
87+
THEN("Every still-enqueued element has its destructor run") {
88+
CHECK(queue_live_tracker_count().load(std::memory_order_relaxed) == 0);
89+
}
90+
}
91+
}
92+
}
93+
94+
/// Const lvalue enqueue delivers the same value on dequeue.
95+
template <typename Queue>
96+
void copy_enqueue_const_payload_scenario() {
97+
GIVEN("An empty queue") {
98+
Queue queue;
99+
100+
WHEN("A value is enqueued via the const lvalue overload") {
101+
const int value = 7;
102+
queue.enqueue(value);
103+
104+
THEN("The same value is dequeued") {
105+
int out = 0;
106+
CHECK(queue.try_dequeue(out));
107+
CHECK(out == 7);
108+
detail::assert_queue_reports_empty(queue);
109+
}
110+
}
111+
}
112+
}
113+
114+
/// Single-threaded FIFO enqueue then dequeue.
115+
template <typename Queue>
116+
void single_producer_consumer_fifo_scenario() {
117+
GIVEN("An empty queue") {
118+
Queue queue;
119+
120+
WHEN("Two values are enqueued in order") {
121+
queue.enqueue(1);
122+
queue.enqueue(2);
123+
124+
THEN("They are dequeued in the same order and the queue is then empty") {
125+
int value = 0;
126+
CHECK(queue.try_dequeue(value));
127+
CHECK(value == 1);
128+
CHECK(queue.try_dequeue(value));
129+
CHECK(value == 2);
130+
detail::assert_queue_reports_empty(queue);
131+
}
132+
}
133+
}
134+
}
135+
136+
/// Move-only payloads can be enqueued and dequeued without copying.
137+
template <typename Queue>
138+
void move_only_payload_scenario() {
139+
GIVEN("A queue of std::unique_ptr<int>") {
140+
Queue queue;
141+
142+
WHEN("A unique_ptr holding 42 is enqueued") {
143+
queue.enqueue(std::make_unique<int>(42));
144+
145+
THEN("The same value can be dequeued without copying") {
146+
std::unique_ptr<int> value;
147+
CHECK(queue.try_dequeue(value));
148+
REQUIRE(value != nullptr);
149+
CHECK(*value == 42);
150+
}
151+
}
152+
}
153+
}
154+
155+
/// Many sequential enqueues followed by many dequeues preserve order.
156+
template <typename Queue>
157+
void sequential_enqueue_dequeue_scenario() {
158+
GIVEN("A queue with 5000 sequentially enqueued integers") {
159+
Queue queue;
160+
for (int i = 0; i < 5000; ++i) {
161+
queue.enqueue(i);
162+
}
163+
164+
WHEN("They are all dequeued in turn") {
165+
bool sequence_holds = true;
166+
for (int i = 0; i < 5000; ++i) {
167+
int value = -1;
168+
if (!queue.try_dequeue(value) || value != i) {
169+
sequence_holds = false;
170+
break;
171+
}
172+
}
173+
174+
THEN("Each dequeue returns the next integer in order and the queue is empty") {
175+
CHECK(sequence_holds);
176+
detail::assert_queue_reports_empty(queue);
177+
}
178+
}
179+
}
180+
}
181+
182+
/// Producer and consumer race across a full block boundary without losing order.
183+
template <typename Queue>
184+
void block_boundary_producer_consumer_race_scenario() {
185+
GIVEN("A queue with one full block and a producer about to overflow it") {
186+
Queue queue;
187+
for (int i = 0; i < 64; ++i) {
188+
queue.enqueue(i);
189+
}
190+
191+
WHEN("A producer and consumer race across the block boundary") {
192+
std::atomic<bool> producer_done{false};
193+
std::thread producer([&] {
194+
for (int i = 64; i < 128; ++i) {
195+
queue.enqueue(i);
196+
}
197+
producer_done.store(true, std::memory_order_release);
198+
});
199+
200+
bool in_order = true;
201+
for (int expected = 0; expected < 128; ++expected) {
202+
int value = -1;
203+
while (!queue.try_dequeue(value)) {
204+
std::this_thread::yield();
205+
}
206+
if (value != expected) {
207+
in_order = false;
208+
break;
209+
}
210+
}
211+
212+
producer.join();
213+
214+
THEN("Every integer is delivered in order despite the block rollover race") {
215+
CHECK(producer_done.load(std::memory_order_acquire));
216+
CHECK(in_order);
217+
detail::assert_queue_reports_empty(queue);
218+
}
219+
}
220+
}
221+
}
222+
223+
} // namespace queue_bdd
224+
} // namespace test_util
225+
226+
#endif // TEST_UTIL_QUEUE_BDD_HELPERS_HPP

‎tests/tests/threading/MPSCQueue.cpp‎

Lines changed: 7 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
#include <utility>
2929
#include <vector>
3030

31-
#include "test_util/queue_live_tracker.hpp"
31+
#include "test_util/queue_bdd_helpers.hpp"
3232

3333
namespace NUClear {
3434
namespace threading {
@@ -37,155 +37,30 @@ namespace threading {
3737

3838
SCENARIO("An MPSCQueue destroyed while non-empty runs the destructors of its remaining items",
3939
"[threading][queue][MPSCQueue]") {
40-
GIVEN("An MPSCQueue filled across several blocks then only partially drained") {
41-
test_util::queue_live_tracker_count().store(0, std::memory_order_relaxed);
42-
43-
WHEN("The queue is destroyed with items still enqueued") {
44-
{
45-
MPSCQueue<test_util::QueueLiveTracker> queue;
46-
for (int i = 0; i < 200; ++i) {
47-
queue.enqueue(test_util::QueueLiveTracker(i));
48-
}
49-
/*drain a few*/ {
50-
test_util::QueueLiveTracker sink(-1);
51-
for (int i = 0; i < 10; ++i) {
52-
REQUIRE(queue.try_dequeue(sink));
53-
}
54-
}
55-
56-
// 190 elements remain live inside the queue's blocks.
57-
CHECK(test_util::queue_live_tracker_count().load(std::memory_order_relaxed) == 190);
58-
}
59-
60-
THEN("Every still-enqueued element has its destructor run") {
61-
CHECK(test_util::queue_live_tracker_count().load(std::memory_order_relaxed) == 0);
62-
}
63-
}
64-
}
40+
test_util::queue_bdd::destructor_runs_remaining_destructors_scenario<MPSCQueue<test_util::QueueLiveTracker>>();
6541
}
6642

6743
SCENARIO("An MPSCQueue accepts copy-enqueued const payloads", "[threading][queue][MPSCQueue]") {
68-
GIVEN("An empty MPSCQueue<int>") {
69-
MPSCQueue<int> queue;
70-
71-
WHEN("A value is enqueued via the const lvalue overload") {
72-
const int value = 7;
73-
queue.enqueue(value);
74-
75-
THEN("The same value is dequeued") {
76-
int out = 0;
77-
CHECK(queue.try_dequeue(out));
78-
CHECK(out == 7);
79-
CHECK_FALSE(queue.try_dequeue(out));
80-
}
81-
}
82-
}
44+
test_util::queue_bdd::copy_enqueue_const_payload_scenario<MPSCQueue<int>>();
8345
}
8446

8547
SCENARIO("An MPSCQueue consumer waits while a producer links the next block",
8648
"[threading][queue][MPSCQueue]") {
87-
GIVEN("An MPSCQueue with one full block and a producer about to overflow it") {
88-
MPSCQueue<int> queue;
89-
for (int i = 0; i < 64; ++i) {
90-
queue.enqueue(i);
91-
}
92-
93-
WHEN("A producer and consumer race across the block boundary") {
94-
std::atomic<bool> producer_done{false};
95-
std::thread producer([&] {
96-
for (int i = 64; i < 128; ++i) {
97-
queue.enqueue(i);
98-
}
99-
producer_done.store(true, std::memory_order_release);
100-
});
101-
102-
bool in_order = true;
103-
for (int expected = 0; expected < 128; ++expected) {
104-
int value = -1;
105-
while (!queue.try_dequeue(value)) {
106-
std::this_thread::yield();
107-
}
108-
if (value != expected) {
109-
in_order = false;
110-
break;
111-
}
112-
}
113-
114-
producer.join();
115-
116-
THEN("Every integer is delivered in order despite the block rollover race") {
117-
CHECK(producer_done.load(std::memory_order_acquire));
118-
CHECK(in_order);
119-
int discard = 0;
120-
CHECK_FALSE(queue.try_dequeue(discard));
121-
}
122-
}
123-
}
49+
test_util::queue_bdd::block_boundary_producer_consumer_race_scenario<MPSCQueue<int>>();
12450
}
12551

12652
SCENARIO("An MPSCQueue used by a single producer and single consumer preserves FIFO order",
12753
"[threading][queue][MPSCQueue]") {
128-
GIVEN("An empty MPSCQueue<int>") {
129-
MPSCQueue<int> queue;
130-
131-
WHEN("Two values are enqueued in order") {
132-
queue.enqueue(1);
133-
queue.enqueue(2);
134-
135-
THEN("They are dequeued in the same order and the queue is then empty") {
136-
int value = 0;
137-
CHECK(queue.try_dequeue(value));
138-
CHECK(value == 1);
139-
CHECK(queue.try_dequeue(value));
140-
CHECK(value == 2);
141-
CHECK_FALSE(queue.try_dequeue(value));
142-
}
143-
}
144-
}
54+
test_util::queue_bdd::single_producer_consumer_fifo_scenario<MPSCQueue<int>>();
14555
}
14656

14757
SCENARIO("An MPSCQueue can store move-only payloads", "[threading][queue][MPSCQueue]") {
148-
GIVEN("An MPSCQueue of std::unique_ptr<int>") {
149-
MPSCQueue<std::unique_ptr<int>> queue;
150-
151-
WHEN("A unique_ptr holding 42 is enqueued") {
152-
queue.enqueue(std::make_unique<int>(42));
153-
154-
THEN("The same value can be dequeued without copying") {
155-
std::unique_ptr<int> value;
156-
CHECK(queue.try_dequeue(value));
157-
REQUIRE(value != nullptr);
158-
CHECK(*value == 42);
159-
}
160-
}
161-
}
58+
test_util::queue_bdd::move_only_payload_scenario<MPSCQueue<std::unique_ptr<int>>>();
16259
}
16360

16461
SCENARIO("An MPSCQueue handles many enqueues from one thread followed by many dequeues",
16562
"[threading][queue][MPSCQueue]") {
166-
GIVEN("An MPSCQueue with 5000 sequentially enqueued integers") {
167-
MPSCQueue<int> queue;
168-
for (int i = 0; i < 5000; ++i) {
169-
queue.enqueue(i);
170-
}
171-
172-
WHEN("They are all dequeued in turn") {
173-
bool sequence_holds = true;
174-
for (int i = 0; i < 5000; ++i) {
175-
int value = -1;
176-
if (!queue.try_dequeue(value) || value != i) {
177-
sequence_holds = false;
178-
break;
179-
}
180-
}
181-
182-
THEN("Each dequeue returns the next integer in order and the queue is empty") {
183-
CHECK(sequence_holds);
184-
int discard = 0;
185-
CHECK_FALSE(queue.try_dequeue(discard));
186-
}
187-
}
188-
}
63+
test_util::queue_bdd::sequential_enqueue_dequeue_scenario<MPSCQueue<int>>();
18964
}
19065

19166
// Stress test for the MPSC contract: many producers race to enqueue while a single consumer

0 commit comments

Comments
 (0)