Use virtual time in TaskQueue unit tests

[gavv]

Last revised: Oct 2023

Update: The task is still relevant. Description is updated to address problems discussed in unfinished PR, linked below.

Currently unit tests for ctl::ControlTaskQueue (test_task_queue.cpp) are partially bound to real clock time (core::timestamp() and core::Timer) and due to this they can fail on loaded system, e.g. slow CI runners.

We can make them 100% reliable by using virtual clock instead of real time, i.e. clock that is fully controlled by unit tests.

We already use such approach in unit tests for pipeline::PipelineLoop (test_pipeline_loop.cpp). PipelineLoop doesn't call core::timestamp() directly. Instead, it have a protected virtual method to get time:

virtual core::nanoseconds_t timestamp_imp() const

In test_pipeline_loop.cpp we inherit PipelineLoop, override this method to return virtual (fake) time, and test this derived class instead of original PipelineLoop.

Now we could use the same approach test_task_queue.cpp:

• In ctl::ControlTaskQueue, add virtual methods that isolate calls to core::timestamp() and core::Timer

  For example, we can add 3 methods:

  • core::nanoseconds_t timestamp_imp()
  • bool try_set_deadline(nanoseconds_t)
  • void wait_deadline()

  Default implementations will just call core::timestamp() and Timer methods.

• In test_task_queue.cpp, add a class that inherits ctl::ControlTaskQueue, overrides these 3 methods to work with virtual/fake time, and adds helpers to control that fake time.

• Adjust tests to work with virtual time instead of real time, i.e. don't use sleep and instead call helpers to control fake time.

[minhdb]

Hi, can I work on this issue?

[gavv]

Sure, you're welcome!

[minhdb]

I have several questions:

1. I have added a protected virtual method timestamp_imp() to ctl::TaskQueue. Since ctl::TestTaskQueue is a derived class, it needs to implement timestamp_imp(). Similar to pipeline::TestPipeLine, I added a public field time_ and set_time() to ctl::TestTaskQueue. Am I in the right direction?

2. ctl::TestHandler has this public method

void expect_after(core::nanoseconds_t delay)
 {
     expect_after_ = timestamp() + delay;
 }

Initially, I planned to replace timestamp() with a task_.timestamp_imp() but task_ could be null so it doesn't seem viable. For reference, task_ is a public field of TaskHandler with type TaskQueue::Task*

Do I add a public field time_ and set_time() to ctl::TestHandler, similar to pipeline::TestPipeLine? The problem with this approach is we have to manually pass reasonable values to handler.expect_after() in the unit tests.

3. What are the parameters, other than StartTime, that contribute to the allowed task queue processing time for TestTaskQueue? For example, in test_task_pipeline.cpp, they are MaxInframeProcessing, NoTaskProcessingGap, StartTime, and FrameProcessingTime.

Thank you!

[minhdb]

I think I got it but it seems like my solution's very hacky. It's basically manipulating the virtual clock to pass the task_queue tests. Especially for tests like schedule_at_shuffled or schedule_and_async_cancel.

Right now it has passed all the tests. But with the cost of several lines code to set the virtual time clock to the task's scheduled time whenever wait_called() is called.

I will be playing around, cleaning up, and doing more testing before issuing a pull request. Also, is there a need for an option that allows user to pick whether to use real / virtual time testing? Please let me know what you think.

[gavv]

I have added a protected virtual method timestamp_imp() to ctl::TaskQueue. Since ctl::TestTaskQueue is a derived class, it needs to implement timestamp_imp(). Similar to pipeline::TestPipeLine, I added a public field time_ and set_time() to ctl::TestTaskQueue. Am I in the right direction?

Yes.

Do I add a public field time_ and set_time() to ctl::TestHandler, similar to pipeline::TestPipeLine? The problem with this approach is we have to manually pass reasonable values to handler.expect_after() in the unit tests.

TestHandler uses core::timestamp() to check when TaskQueue actually calls the handler. So with virtual clock approach, I think the most clean way would be to make TestHandler checking the current (virtual) time in TaskQueue instead of having its own time.

We could pass a reference to TestTaskQueue to TestHandler ctor, so that it can call TestTaskQueue::get_time(). Or we can extract virtual clock into separate class (e.g. TestClock), with get_time() and set_time() methods, and pass it to both TestTaskQueue and TestHandler.

What are the parameters, other than StartTime, that contribute to the allowed task queue processing time for TestTaskQueue?

IIRC only StartTime and per-task deadline matter.

I think I got it but it seems like my solution's very hacky. It's basically manipulating the virtual clock to pass the task_queue tests. Especially for tests like schedule_at_shuffled or schedule_and_async_cancel.

In addition to getting rid of TestHandler::set_time (see above), we could also replace TestTaskQueue::set_time() with TestTaskQueue::add_time(), i.e. pass delta instead of abs value. I think this will make tests cleaner or at least more similar to original tests with sleep calls.

Also, is there a need for an option that allows user to pick whether to use real / virtual time testing?

I think no. We should use virtual time in tests and real time in benchmarks.

[gavv]

Unassigned so that someone could pick it up again. See also discussion in PR linked above regarding core::Timer: #437

UPD: I've updated task description to address what was discussed in PR.