Skip to content

Commit d7557cc

Browse files
allnesallnes
committed
refactor task framework: rename and extend lifecycle handling for better test coverage and clarity
1 parent 24a935a commit d7557cc

File tree

7 files changed

+64
-68
lines changed

7 files changed

+64
-68
lines changed

‎.github/workflows/static-analysis-pr.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ jobs:
7575
runs-on: ubuntu-24.04
7676
steps:
7777
- uses: actions/checkout@v4
78-
- name: Search for linter suppression markers
78+
- name: Search for forbidden patterns in student tasks
7979
run: |
8080
export BASE_REF=${{ github.event.pull_request.base.ref }}
8181
export CHANGED_FILES="$(git diff --name-only origin/$BASE_REF HEAD | grep '^tasks/')"
@@ -92,5 +92,9 @@ jobs:
9292
echo "::error::Found 'IWYU pragma' in $file."
9393
exit 1
9494
fi
95+
if grep -n "ExpectIncompleteLifecycle" "$file"; then
96+
echo "::error::Found 'ExpectIncompleteLifecycle' in $file. This function is for internal testing only and should not be used in student tasks."
97+
exit 1
98+
fi
9599
done
96-
echo "No linter suppression markers found in changed files."
100+
echo "No forbidden patterns found in changed files."

‎modules/performance/tests/perf_tests.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ TEST(TaskTest, Destructor_WithWrongOrder_TerminatesGracefully) {
282282
{
283283
DummyTask task;
284284
EXPECT_THROW(task.Run(), std::runtime_error);
285-
// This task doesn't cause destructor failure - just execution order error
285+
// This task doesn't cause destructor failure - just an execution order error
286286
}
287287

288288
// Create a new task to complete the lifecycle properly
@@ -356,11 +356,10 @@ TEST(PerfTest, PipelineRunAndTaskRun_WithValidTask_ExecutesSuccessfully) {
356356
TEST(PerfTest, PrintPerfStatistic_WithNoneType_ThrowsException) {
357357
{
358358
auto task_ptr = std::make_shared<DummyTask>();
359+
task_ptr->ExpectIncompleteLifecycle(); // Task not executed in a performance test
359360
Perf<int, int> perf(task_ptr);
360361
EXPECT_THROW(perf.PrintPerfStatistic("test"), std::runtime_error);
361362
}
362-
EXPECT_TRUE(ppc::util::DestructorFailureFlag::Get());
363-
ppc::util::DestructorFailureFlag::Unset();
364363
}
365364

366365
TEST(PerfTest, GetStringParamName_WithValidParameters_ReturnsCorrectString) {
@@ -617,10 +616,9 @@ TEST(TaskTest, Destructor_WithInvalidPipelineOrderAndPartialExecution_Terminates
617616
bool RunImpl() override { return true; }
618617
bool PostProcessingImpl() override { return true; }
619618
} task;
619+
task.ExpectIncompleteLifecycle(); // Task has incomplete pipeline execution
620620
task.Validation();
621621
}
622-
EXPECT_TRUE(ppc::util::DestructorFailureFlag::Get());
623-
ppc::util::DestructorFailureFlag::Unset();
624622
}
625623

626624
} // namespace

‎modules/runners/src/runners.cpp

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,6 @@ void WorkerTestFailurePrinter::PrintProcessRank() {
6060
std::cerr << std::format(" [ PROCESS {} ] ", rank);
6161
}
6262

63-
namespace {
64-
int RunAllTests() {
65-
auto status = RUN_ALL_TESTS();
66-
if (ppc::util::DestructorFailureFlag::Get()) {
67-
throw std::runtime_error(
68-
std::format("[ ERROR ] Destructor failed with code {}", ppc::util::DestructorFailureFlag::Get()));
69-
}
70-
return status;
71-
}
72-
} // namespace
73-
7463
int Init(int argc, char** argv) {
7564
const int init_res = MPI_Init(&argc, &argv);
7665
if (init_res != MPI_SUCCESS) {
@@ -79,21 +68,25 @@ int Init(int argc, char** argv) {
7968
return init_res;
8069
}
8170

82-
// Limit the number of threads in TBB
83-
tbb::global_control control(tbb::global_control::max_allowed_parallelism, ppc::util::GetNumThreads());
71+
auto status = 0;
72+
{
73+
// Limit the number of threads in TBB
74+
tbb::global_control control(tbb::global_control::max_allowed_parallelism, ppc::util::GetNumThreads());
8475

85-
::testing::InitGoogleTest(&argc, argv);
76+
::testing::InitGoogleTest(&argc, argv);
8677

87-
auto& listeners = ::testing::UnitTest::GetInstance()->listeners();
88-
int rank = -1;
89-
MPI_Comm_rank(MPI_COMM_WORLD, &rank);
90-
if (rank != 0 && (argc < 2 || argv[1] != std::string("--print-workers"))) {
91-
auto* listener = listeners.Release(listeners.default_result_printer());
92-
listeners.Append(new WorkerTestFailurePrinter(std::shared_ptr<::testing::TestEventListener>(listener)));
93-
}
94-
listeners.Append(new UnreadMessagesDetector());
78+
auto& listeners = ::testing::UnitTest::GetInstance()->listeners();
79+
int rank = -1;
80+
MPI_Comm_rank(MPI_COMM_WORLD, &rank);
81+
if (rank != 0 && (argc < 2 || argv[1] != std::string("--print-workers"))) {
82+
auto* listener = listeners.Release(listeners.default_result_printer());
83+
listeners.Append(new WorkerTestFailurePrinter(std::shared_ptr<::testing::TestEventListener>(listener)));
84+
}
85+
listeners.Append(new UnreadMessagesDetector());
86+
87+
status = RUN_ALL_TESTS();
88+
} // TBB control object destroyed here
9589

96-
auto status = RunAllTests();
9790

9891
const int finalize_res = MPI_Finalize();
9992
if (finalize_res != MPI_SUCCESS) {
@@ -105,11 +98,16 @@ int Init(int argc, char** argv) {
10598
}
10699

107100
int SimpleInit(int argc, char** argv) {
108-
// Limit the number of threads in TBB
109-
tbb::global_control control(tbb::global_control::max_allowed_parallelism, ppc::util::GetNumThreads());
101+
auto status = 0;
102+
{
103+
// Limit the number of threads in TBB
104+
tbb::global_control control(tbb::global_control::max_allowed_parallelism, ppc::util::GetNumThreads());
110105

111-
testing::InitGoogleTest(&argc, argv);
112-
return RunAllTests();
106+
testing::InitGoogleTest(&argc, argv);
107+
status = RUN_ALL_TESTS();
108+
} // TBB control object destroyed here
109+
110+
return status;
113111
}
114112

115113
} // namespace ppc::runners

‎modules/task/include/task.hpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,20 @@ class Task {
190190
/// @return Reference to the task's output data.
191191
OutType &GetOutput() { return output_; }
192192

193+
/// @brief Marks that this task is expected to have an incomplete lifecycle.
194+
/// @note FOR INTERNAL TESTING ONLY. This function should NOT be used in student tasks.
195+
/// Usage in tasks/ directory will cause CI to fail.
196+
/// @warning This function is only for framework testing purposes.
197+
void ExpectIncompleteLifecycle() { expect_incomplete_ = true; }
198+
193199
/// @brief Destructor. Verifies that the pipeline was executed in the correct order.
194200
/// @note Terminates the program if the pipeline order is incorrect or incomplete.
195201
virtual ~Task() {
196-
if (stage_ != PipelineStage::kDone && stage_ != PipelineStage::kException) {
197-
ppc::util::DestructorFailureFlag::Set();
202+
if (stage_ != PipelineStage::kDone && stage_ != PipelineStage::kException && !expect_incomplete_) {
203+
// Immediate failure - better than global state pollution
204+
std::cerr << "[TASK ERROR] Task destroyed without completing pipeline. Stage: "
205+
<< static_cast<int>(stage_) << std::endl;
206+
std::terminate();
198207
}
199208
#if _OPENMP >= 201811
200209
omp_pause_resource_all(omp_pause_soft);
@@ -259,6 +268,7 @@ class Task {
259268
kDone,
260269
kException
261270
} stage_ = PipelineStage::kNone;
271+
bool expect_incomplete_ = false; // Allow testing of incomplete pipelines
262272
};
263273

264274
/// @brief Smart pointer alias for Task.

‎modules/task/tests/task_additional.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ TEST_F(TaskAdditionalTest, TaskGetter_WithBasicTask_CreatesTaskSuccessfully) {
3838

3939
// Test TaskGetter function
4040
auto getter_result = ppc::task::TaskGetter<GetterTestTask>(42);
41+
getter_result->ExpectIncompleteLifecycle(); // Task is only created for testing, not executed
4142

4243
EXPECT_NE(getter_result, nullptr);
4344
EXPECT_EQ(getter_result->GetValue(), 42);
@@ -78,6 +79,9 @@ TEST_F(TaskAdditionalTest, TaskGetter_WithDifferentTaskTypes_CreatesTasksSuccess
7879

7980
auto getter1 = ppc::task::TaskGetter<TaskType1>(std::string("test"));
8081
auto getter2 = ppc::task::TaskGetter<TaskType2>(3.14);
82+
83+
getter1->ExpectIncompleteLifecycle(); // Tasks are only created for testing
84+
getter2->ExpectIncompleteLifecycle(); // Tasks are only created for testing
8185

8286
EXPECT_NE(getter1, nullptr);
8387
EXPECT_NE(getter2, nullptr);

‎modules/task/tests/task_tests.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,21 @@ TEST(TaskTest, SlowTask_WithInt32Vector_ThrowsOnTimeout) {
8383
{
8484
std::vector<int32_t> in(20, 1);
8585
ppc::test::FakeSlowTask<std::vector<int32_t>, int32_t> test_task(in);
86+
test_task.ExpectIncompleteLifecycle(); // Task may not complete due to timeout
8687
ASSERT_EQ(test_task.Validation(), true);
8788
test_task.PreProcessing();
8889
test_task.Run();
8990
ASSERT_ANY_THROW(test_task.PostProcessing());
9091
}
91-
ppc::util::DestructorFailureFlag::Unset();
9292
}
9393

9494
TEST(TaskTest, TestTask_WithEmptyInput_ValidationFails) {
9595
{
9696
std::vector<int32_t> in;
9797
ppc::test::TestTask<std::vector<int32_t>, int32_t> test_task(in);
98+
test_task.ExpectIncompleteLifecycle(); // Task fails validation so won't complete
9899
ASSERT_EQ(test_task.Validation(), false);
99100
}
100-
ppc::util::DestructorFailureFlag::Unset();
101101
}
102102

103103
TEST(TaskTest, TestTask_WithDoubleVector_CompletesSuccessfully) {
@@ -124,30 +124,30 @@ TEST(TaskTest, TestTask_WithWrongExecutionOrder_ThrowsRuntimeError) {
124124
{
125125
std::vector<float> in(20, 1);
126126
ppc::test::TestTask<std::vector<float>, float> test_task(in);
127+
test_task.ExpectIncompleteLifecycle(); // Task has wrong execution order
127128
ASSERT_EQ(test_task.Validation(), true);
128129
test_task.PreProcessing();
129130
EXPECT_THROW(test_task.PostProcessing(), std::runtime_error);
130131
}
131-
ppc::util::DestructorFailureFlag::Unset();
132132
}
133133

134134
TEST(TaskTest, TestTask_WithPrematurePostProcessingNoSteps_ThrowsRuntimeError) {
135135
{
136136
std::vector<float> in(20, 1);
137137
ppc::test::TestTask<std::vector<float>, float> test_task(in);
138+
test_task.ExpectIncompleteLifecycle(); // Task throws exception so won't complete
138139
EXPECT_THROW(test_task.PostProcessing(), std::runtime_error);
139140
}
140-
ppc::util::DestructorFailureFlag::Unset();
141141
}
142142

143143
TEST(TaskTest, TestTask_WithPrematurePostProcessingAfterPreProcessing_ThrowsRuntimeError) {
144144
{
145145
std::vector<float> in(20, 1);
146146
ppc::test::TestTask<std::vector<float>, float> test_task(in);
147+
test_task.ExpectIncompleteLifecycle(); // Task throws exceptions so won't complete
147148
EXPECT_THROW(test_task.PreProcessing(), std::runtime_error);
148149
EXPECT_THROW(test_task.PostProcessing(), std::runtime_error);
149150
}
150-
ppc::util::DestructorFailureFlag::Unset();
151151
}
152152

153153
TEST(TaskTest, GetStringTaskStatus_WithDisabledStatus_ReturnsDisabled) {
@@ -228,10 +228,10 @@ TEST(TaskTest, TaskDestructor_WithIncompleteStage_SetsDestructorFailureFlag) {
228228
bool RunImpl() override { return true; }
229229
bool PostProcessingImpl() override { return true; }
230230
} task(in);
231+
task.ExpectIncompleteLifecycle(); // Mark this task as expected to be incomplete
231232
task.Validation();
232233
}
233-
EXPECT_TRUE(ppc::util::DestructorFailureFlag::Get());
234-
ppc::util::DestructorFailureFlag::Unset();
234+
// No need to check global flag - task handles its own validation
235235
}
236236

237237
TEST(TaskTest, TaskDestructor_WithEmptyTask_SetsDestructorFailureFlag) {
@@ -244,9 +244,9 @@ TEST(TaskTest, TaskDestructor_WithEmptyTask_SetsDestructorFailureFlag) {
244244
bool RunImpl() override { return true; }
245245
bool PostProcessingImpl() override { return true; }
246246
} task(in);
247+
task.ExpectIncompleteLifecycle(); // Mark this task as expected to be incomplete
247248
}
248-
EXPECT_TRUE(ppc::util::DestructorFailureFlag::Get());
249-
ppc::util::DestructorFailureFlag::Unset();
249+
// No need to check global flag - task handles its own validation
250250
}
251251

252252
TEST(TaskTest, InternalTimeTest_WithTimeoutExceeded_ThrowsRuntimeError) {
@@ -264,13 +264,13 @@ TEST(TaskTest, InternalTimeTest_WithTimeoutExceeded_ThrowsRuntimeError) {
264264
{
265265
std::vector<int32_t> in(20, 1);
266266
SlowTask task(in);
267+
task.ExpectIncompleteLifecycle(); // Task throws timeout exception
267268
task.GetStateOfTesting() = StateOfTesting::kFunc;
268269
task.Validation();
269270
EXPECT_NO_THROW(task.PreProcessing());
270271
task.Run();
271272
EXPECT_THROW(task.PostProcessing(), std::runtime_error);
272273
}
273-
ppc::util::DestructorFailureFlag::Unset();
274274
}
275275

276276
class DummyTask : public Task<int, int> {
@@ -285,36 +285,36 @@ class DummyTask : public Task<int, int> {
285285
TEST(TaskTest, Validation_WhenCalledTwice_ThrowsRuntimeError) {
286286
{
287287
auto task = std::make_shared<DummyTask>();
288+
task->ExpectIncompleteLifecycle(); // Task throws exception so won't complete
288289
task->Validation();
289290
EXPECT_THROW(task->Validation(), std::runtime_error);
290291
}
291-
ppc::util::DestructorFailureFlag::Unset();
292292
}
293293

294294
TEST(TaskTest, PreProcessing_WhenCalledBeforeValidation_ThrowsRuntimeError) {
295295
{
296296
auto task = std::make_shared<DummyTask>();
297+
task->ExpectIncompleteLifecycle(); // Task throws exception so won't complete
297298
EXPECT_THROW(task->PreProcessing(), std::runtime_error);
298299
}
299-
ppc::util::DestructorFailureFlag::Unset();
300300
}
301301

302302
TEST(TaskTest, Run_WhenCalledBeforePreProcessing_ThrowsRuntimeError) {
303303
{
304304
auto task = std::make_shared<DummyTask>();
305+
task->ExpectIncompleteLifecycle(); // Task throws exception so won't complete
305306
EXPECT_THROW(task->Run(), std::runtime_error);
306307
}
307-
ppc::util::DestructorFailureFlag::Unset();
308308
}
309309

310310
TEST(TaskTest, PostProcessing_WhenCalledBeforeRun_ThrowsRuntimeError) {
311311
{
312312
auto task = std::make_shared<DummyTask>();
313+
task->ExpectIncompleteLifecycle(); // Task throws exception so won't complete
313314
task->Validation();
314315
task->PreProcessing();
315316
EXPECT_THROW(task->PostProcessing(), std::runtime_error);
316317
}
317-
ppc::util::DestructorFailureFlag::Unset();
318318
}
319319

320320
int main(int argc, char** argv) { return ppc::runners::SimpleInit(argc, argv); }

‎modules/util/include/util.hpp

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#pragma once
22

3-
#include <atomic>
43
#include <cstdint>
54
#include <cstdlib>
65
#include <memory>
@@ -30,23 +29,6 @@ using NlohmannJsonTypeError = nlohmann::json::type_error;
3029

3130
namespace ppc::util {
3231

33-
/// @brief Utility class for tracking destructor failure across tests.
34-
/// @details Provides thread-safe methods to set, unset, and check the failure flag.
35-
class DestructorFailureFlag {
36-
public:
37-
/// @brief Marks that a destructor failure has occurred.
38-
static void Set() { failure_flag.store(true); }
39-
40-
/// @brief Clears the destructor failure flag.
41-
static void Unset() { failure_flag.store(false); }
42-
43-
/// @brief Checks if a destructor failure was recorded.
44-
/// @return True if failure occurred, false otherwise.
45-
static bool Get() { return failure_flag.load(); }
46-
47-
private:
48-
inline static std::atomic<bool> failure_flag{false};
49-
};
5032

5133
enum GTestParamIndex : uint8_t { kTaskGetter, kNameTest, kTestParams };
5234

0 commit comments

Comments
 (0)