From 35a35d99e95523ec61c506a9f90094cfa708e768 Mon Sep 17 00:00:00 2001 From: Per Malmberg Date: Tue, 13 Mar 2018 02:37:31 -0700 Subject: [PATCH] Handles jumps backwards backwards >3h --- .gitignore | 3 +- libcron/Cron.h | 8 +- libcron/Task.cpp | 5 +- libcron/Task.h | 4 +- test/CronTest.cpp | 367 ++++++++++++++++++++++++---------------------- 5 files changed, 205 insertions(+), 182 deletions(-) diff --git a/.gitignore b/.gitignore index 8b4163a..d97585d 100644 --- a/.gitignore +++ b/.gitignore @@ -32,4 +32,5 @@ *.app cmake-build-* -.idea/workspace.xml \ No newline at end of file +.idea/workspace.xml +out/* \ No newline at end of file diff --git a/libcron/Cron.h b/libcron/Cron.h index 252a20d..0c2e54b 100644 --- a/libcron/Cron.h +++ b/libcron/Cron.h @@ -122,12 +122,12 @@ namespace libcron t.calculate_next(now); } } - else if(now < last_tick && now - last_tick < hours{3}) + else if (now < last_tick && now - last_tick <= -hours{3}) { - // Prevent tasks from running until the clock has reached current 'last_tick'. + // Reschedule all tasks. for (auto& t : tasks.get_tasks()) { - //t.set_back_limit(last_tick); + t.calculate_next(now); } } @@ -141,7 +141,7 @@ namespace libcron executed.push_back(tasks.top()); tasks.pop(); auto& t = executed[executed.size() - 1]; - t.execute(); + t.execute(now); } res = executed.size(); diff --git a/libcron/Task.cpp b/libcron/Task.cpp index 0171cf8..fc0b038 100644 --- a/libcron/Task.cpp +++ b/libcron/Task.cpp @@ -15,6 +15,9 @@ namespace libcron if (valid) { next_schedule = std::get<1>(result); + + // Make sure that the task is allowed to run. + last_run = next_schedule - 1s; } return valid; @@ -22,7 +25,7 @@ namespace libcron bool Task::is_expired(std::chrono::system_clock::time_point now) const { - return valid && time_until_expiry(now) == 0s; + return valid && now >= last_run && time_until_expiry(now) == 0s; } std::chrono::system_clock::duration Task::time_until_expiry(std::chrono::system_clock::time_point now) const diff --git a/libcron/Task.h b/libcron/Task.h index f44b03c..b0a918d 100644 --- a/libcron/Task.h +++ b/libcron/Task.h @@ -17,8 +17,9 @@ namespace libcron { } - void execute() const + void execute(std::chrono::system_clock::time_point now) { + last_run = now; task(); } @@ -51,5 +52,6 @@ namespace libcron std::chrono::system_clock::time_point next_schedule; std::function task; bool valid = false; + std::chrono::system_clock::time_point last_run = std::numeric_limits::min(); }; } \ No newline at end of file diff --git a/test/CronTest.cpp b/test/CronTest.cpp index cda69a3..51fa00b 100644 --- a/test/CronTest.cpp +++ b/test/CronTest.cpp @@ -19,177 +19,177 @@ std::string create_schedule_expiring_in(std::chrono::system_clock::time_point no return res; } -//SCENARIO("Adding a task") -//{ -// GIVEN("A Cron instance with no task") -// { -// Cron<> c; -// auto expired = false; -// -// THEN("Starts with no task") -// { -// REQUIRE(c.count() == 0); -// } -// -// WHEN("Adding a task that runs every second") -// { -// REQUIRE(c.add_schedule("A task", "* * * * * ?", -// [&expired]() -// { -// expired = true; -// }) -// ); -// -// THEN("Count is 1 and task was not expired two seconds ago") -// { -// REQUIRE(c.count() == 1); -// c.tick(c.get_clock().now() - 2s); -// REQUIRE_FALSE(expired); -// } -// AND_THEN("Task is expired when calculating based on current time") -// { -// c.tick(); -// THEN("Task is expired") -// { -// REQUIRE(expired); -// } -// } -// } -// } -//} -// -//SCENARIO("Adding a task that expires in the future") -//{ -// GIVEN("A Cron instance with task expiring in 3 seconds") -// { -// auto expired = false; -// -// Cron<> c; -// REQUIRE(c.add_schedule("A task", -// create_schedule_expiring_in(c.get_clock().now(), hours{0}, minutes{0}, seconds{3}), -// [&expired]() -// { -// expired = true; -// }) -// ); -// -// THEN("Not yet expired") -// { -// REQUIRE_FALSE(expired); -// } -// AND_WHEN("When waiting one second") -// { -// std::this_thread::sleep_for(1s); -// c.tick(); -// THEN("Task has not yet expired") -// { -// REQUIRE_FALSE(expired); -// } -// } -// AND_WHEN("When waiting three seconds") -// { -// std::this_thread::sleep_for(3s); -// c.tick(); -// THEN("Task has expired") -// { -// REQUIRE(expired); -// } -// } -// } -//} -// -//SCENARIO("Task priority") -//{ -// GIVEN("A Cron instance with two tasks expiring in 3 and 5 seconds, added in 'reverse' order") -// { -// auto _3_second_expired = 0; -// auto _5_second_expired = 0; -// -// -// Cron<> c; -// REQUIRE(c.add_schedule("Five", -// create_schedule_expiring_in(c.get_clock().now(), hours{0}, minutes{0}, seconds{5}), -// [&_5_second_expired]() -// { -// _5_second_expired++; -// }) -// ); -// -// REQUIRE(c.add_schedule("Three", -// create_schedule_expiring_in(c.get_clock().now(), hours{0}, minutes{0}, seconds{3}), -// [&_3_second_expired]() -// { -// _3_second_expired++; -// }) -// ); -// -// THEN("Not yet expired") -// { -// REQUIRE_FALSE(_3_second_expired); -// REQUIRE_FALSE(_5_second_expired); -// } -// -// WHEN("Waiting 1 seconds") -// { -// std::this_thread::sleep_for(1s); -// c.tick(); -// -// THEN("Task has not yet expired") -// { -// REQUIRE(_3_second_expired == 0); -// REQUIRE(_5_second_expired == 0); -// } -// } -// AND_WHEN("Waiting 3 seconds") -// { -// std::this_thread::sleep_for(3s); -// c.tick(); -// -// THEN("3 second task has expired") -// { -// REQUIRE(_3_second_expired == 1); -// REQUIRE(_5_second_expired == 0); -// } -// } -// AND_WHEN("Waiting 5 seconds") -// { -// std::this_thread::sleep_for(5s); -// c.tick(); -// -// THEN("3 and 5 second task has expired") -// { -// REQUIRE(_3_second_expired == 1); -// REQUIRE(_5_second_expired == 1); -// } -// } -// AND_WHEN("Waiting based on the time given by the Cron instance") -// { -// std::this_thread::sleep_for(c.time_until_next()); -// c.tick(); -// -// THEN("3 second task has expired") -// { -// REQUIRE(_3_second_expired == 1); -// REQUIRE(_5_second_expired == 0); -// } -// } -// AND_WHEN("Waiting based on the time given by the Cron instance") -// { -// std::this_thread::sleep_for(c.time_until_next()); -// REQUIRE(c.tick() == 1); -// -// std::this_thread::sleep_for(c.time_until_next()); -// REQUIRE(c.tick() == 1); -// -// THEN("3 and 5 second task has each expired once") -// { -// REQUIRE(_3_second_expired == 1); -// REQUIRE(_5_second_expired == 1); -// } -// } -// } -//} -// +SCENARIO("Adding a task") +{ + GIVEN("A Cron instance with no task") + { + Cron<> c; + auto expired = false; + + THEN("Starts with no task") + { + REQUIRE(c.count() == 0); + } + + WHEN("Adding a task that runs every second") + { + REQUIRE(c.add_schedule("A task", "* * * * * ?", + [&expired]() + { + expired = true; + }) + ); + + THEN("Count is 1 and task was not expired two seconds ago") + { + REQUIRE(c.count() == 1); + c.tick(c.get_clock().now() - 2s); + REQUIRE_FALSE(expired); + } + AND_THEN("Task is expired when calculating based on current time") + { + c.tick(); + THEN("Task is expired") + { + REQUIRE(expired); + } + } + } + } +} + +SCENARIO("Adding a task that expires in the future") +{ + GIVEN("A Cron instance with task expiring in 3 seconds") + { + auto expired = false; + + Cron<> c; + REQUIRE(c.add_schedule("A task", + create_schedule_expiring_in(c.get_clock().now(), hours{0}, minutes{0}, seconds{3}), + [&expired]() + { + expired = true; + }) + ); + + THEN("Not yet expired") + { + REQUIRE_FALSE(expired); + } + AND_WHEN("When waiting one second") + { + std::this_thread::sleep_for(1s); + c.tick(); + THEN("Task has not yet expired") + { + REQUIRE_FALSE(expired); + } + } + AND_WHEN("When waiting three seconds") + { + std::this_thread::sleep_for(3s); + c.tick(); + THEN("Task has expired") + { + REQUIRE(expired); + } + } + } +} + +SCENARIO("Task priority") +{ + GIVEN("A Cron instance with two tasks expiring in 3 and 5 seconds, added in 'reverse' order") + { + auto _3_second_expired = 0; + auto _5_second_expired = 0; + + + Cron<> c; + REQUIRE(c.add_schedule("Five", + create_schedule_expiring_in(c.get_clock().now(), hours{0}, minutes{0}, seconds{5}), + [&_5_second_expired]() + { + _5_second_expired++; + }) + ); + + REQUIRE(c.add_schedule("Three", + create_schedule_expiring_in(c.get_clock().now(), hours{0}, minutes{0}, seconds{3}), + [&_3_second_expired]() + { + _3_second_expired++; + }) + ); + + THEN("Not yet expired") + { + REQUIRE_FALSE(_3_second_expired); + REQUIRE_FALSE(_5_second_expired); + } + + WHEN("Waiting 1 seconds") + { + std::this_thread::sleep_for(1s); + c.tick(); + + THEN("Task has not yet expired") + { + REQUIRE(_3_second_expired == 0); + REQUIRE(_5_second_expired == 0); + } + } + AND_WHEN("Waiting 3 seconds") + { + std::this_thread::sleep_for(3s); + c.tick(); + + THEN("3 second task has expired") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 0); + } + } + AND_WHEN("Waiting 5 seconds") + { + std::this_thread::sleep_for(5s); + c.tick(); + + THEN("3 and 5 second task has expired") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 1); + } + } + AND_WHEN("Waiting based on the time given by the Cron instance") + { + std::this_thread::sleep_for(c.time_until_next()); + c.tick(); + + THEN("3 second task has expired") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 0); + } + } + AND_WHEN("Waiting based on the time given by the Cron instance") + { + std::this_thread::sleep_for(c.time_until_next()); + REQUIRE(c.tick() == 1); + + std::this_thread::sleep_for(c.time_until_next()); + REQUIRE(c.tick() == 1); + + THEN("3 and 5 second task has each expired once") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 1); + } + } + } +} + class TestClock : public ICronClock { @@ -261,16 +261,33 @@ SCENARIO("Clock changes") THEN("Task are rescheduled, not run") { REQUIRE(c.tick() == 1); - std::cout << "0: " << c << std::endl; clock.add(hours{3}); // 03:00 REQUIRE(c.tick() == 1); // Rescheduled - std::cout << "1: " << c << std::endl; clock.add(minutes{15}); // 03:15 REQUIRE(c.tick() == 0); - std::cout << "2: " << c << std::endl; clock.add(minutes{45}); // 04:00 REQUIRE(c.tick() == 1); - std::cout << "3: " << c << std::endl; + } + } + AND_WHEN("Clock is moved back <3h") + { + THEN("Tasks retain their last scheduled time and are prevented from running twice") + { + REQUIRE(c.tick() == 1); + clock.add(-hours{1}); // 23:00 + REQUIRE(c.tick() == 0); + } + } + AND_WHEN("Clock is moved back >3h") + { + THEN("Tasks are rescheduled") + { + REQUIRE(c.tick() == 1); + clock.add(-hours{3}); // 23:00 + REQUIRE(c.tick() == 1); + REQUIRE(c.tick() == 0); + clock.add(hours{1}); // 00:00 + REQUIRE(c.tick() == 1); } }