Repository: bitcoin/bitcoin
File: src/test/logging_tests.cpp (C++)

Code before:
        BOOST_CHECK_EQUAL(http_it->second, BCLog::Level::Info);
    }
}

void MockForwardAndSync(CScheduler& scheduler, std::chrono::seconds duration)
{
    scheduler.MockForward(duration);
    std::promise<void> promise;
    scheduler.scheduleFromNow([&promise] { promise.set_value(); }, 0ms);
    promise.get_future().wait();
}

BOOST_AUTO_TEST_CASE(logging_log_rate_limiter)
{
    CScheduler scheduler{};
    scheduler.m_service_thread = std::thread([&scheduler] { scheduler.serviceQueue(); });
    uint64_t max_bytes{1024};
    std::chrono::minutes reset_window{1};
    BCLog::LogRateLimiter::SchedulerFunction sched_func{[&scheduler](std::function<void()> func, std::chrono::milliseconds window) {
        scheduler.scheduleEvery(std::move(func), window);
    }};
    BCLog::LogRateLimiter limiter{sched_func, max_bytes, reset_window};

    using Status = BCLog::LogRateLimiter::Status;
    std::source_location source_loc_1{std::source_location::current()};
    std::source_location source_loc_2{std::source_location::current()};

    // A fresh limiter should not have any suppressions
    BOOST_CHECK(!limiter.SuppressionsActive());

    // Resetting an unused limiter is fine
    limiter.Reset();
    BOOST_CHECK(!limiter.SuppressionsActive());

    // No suppression should happen until more than max_bytes have been consumed
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_1, std::string(max_bytes - 1, 'a')), Status::UNSUPPRESSED);
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_1, "a"), Status::UNSUPPRESSED);
    BOOST_CHECK(!limiter.SuppressionsActive());
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_1, "a"), Status::NEWLY_SUPPRESSED);
    BOOST_CHECK(limiter.SuppressionsActive());
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_1, "a"), Status::STILL_SUPPRESSED);
    BOOST_CHECK(limiter.SuppressionsActive());

    // Location 2 should not be affected by location 1's suppression
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_2, std::string(max_bytes, 'a')), Status::UNSUPPRESSED);
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_2, "a"), Status::NEWLY_SUPPRESSED);
    BOOST_CHECK(limiter.SuppressionsActive());

    // After reset_window time has passed, all suppressions should be cleared.
    MockForwardAndSync(scheduler, reset_window);


Review comment (bug):
nit: `auto` can sometimes make things less readable, but I think in all of these changes that's not the case. Repeating types (e.g. `std::source_location source_loc_1{std::source_location::current()}` is just overly verbose imo), and for the others the name/literal is already explanatory. For `sched_func`, I think the logic is now drowned in rather unimportant type definitions. I think this addresses [this comment](https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2188256330), but I personally don't think there's anything wrong with mixing `auto` and explicit types, we should use whichever is most appropriate at each time imo.

Code after:

struct ScopedScheduler {
    CScheduler scheduler{};

    ScopedScheduler()
    {
        scheduler.m_service_thread = std::thread([this] { scheduler.serviceQueue(); });
    }
    ~ScopedScheduler()
    {
        scheduler.stop();
    }
    void MockForwardAndSync(std::chrono::seconds duration)
    {
        scheduler.MockForward(duration);
        std::promise<void> promise;
        scheduler.scheduleFromNow([&promise] { promise.set_value(); }, 0ms);
        promise.get_future().wait();
    }
    std::shared_ptr<BCLog::LogRateLimiter> GetLimiter(size_t max_bytes, std::chrono::seconds window)
    {
        auto sched_func = [this](auto func, auto w) {
            scheduler.scheduleEvery(std::move(func), w);
        };
        return BCLog::LogRateLimiter::Create(sched_func, max_bytes, window);
    }
};

BOOST_AUTO_TEST_CASE(logging_log_rate_limiter)
{
    uint64_t max_bytes{1024};
    auto reset_window{1min};
    ScopedScheduler scheduler{};
    auto limiter_{scheduler.GetLimiter(max_bytes, reset_window)};
    auto& limiter{*Assert(limiter_)};

    using Status = BCLog::LogRateLimiter::Status;
    auto source_loc_1{std::source_location::current()};
    auto source_loc_2{std::source_location::current()};

    // A fresh limiter should not have any suppressions
    BOOST_CHECK(!limiter.SuppressionsActive());

    // Resetting an unused limiter is fine
    limiter.Reset();
    BOOST_CHECK(!limiter.SuppressionsActive());

    // No suppression should happen until more than max_bytes have been consumed
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_1, std::string(max_bytes - 1, 'a')), Status::UNSUPPRESSED);
    BOOST_CHECK_EQUAL(limiter.Consume(source_loc_1, "a"), Status::UNSUPPRESSED);
    BOOST_CHECK(!limiter.SuppressionsActive());