Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement fake timers for unit tests #1058

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from

Conversation

ggarber
Copy link
Contributor

@ggarber ggarber commented Apr 8, 2023

Implement support for fake timers (timers that run "faster than real time") to make unit tests faster and more predictable.

The approach taken in this PR is to have two implementations of the Timer class. One based on LibUV and one based on a simple map keeping the list of timers (FakeTimerManager). Which one is compiled depends on the FAKE_TIMERS macro being defined that is defined for the test target now.

One of the use cases is to be able to run long tests (f.e. bandwidth estimation tests that take seconds) in milliseconds in unit tests. Existing tests like KeyFrameManagerTests are also faster now.

From a design point of view it also removes the dependency with DevLibUV from many files that should be LibUV agnostic.

Maybe there is a better way to do this, it is open for discussion and I'm happy to take other approach if it makes more sense for mediasoup philosophy or close the PR if it doesn't make sense.

Copy link
Member

@jmillan jmillan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very convenient @ggarber 👍

I'd say FakeTimerManager should live in its own file. Also, can you please add some tests which would serve as documentation for its usage?

@ibc
Copy link
Member

ibc commented Apr 11, 2023

I'll be off for a few days. But as Jose said, please move to a separate file and rename the file and the class to FakeTimeHandle to be in sync with naming changes done in flatbuffers branch.

I don't really know if moving GetTimeXx() methods from Utils to Timer is a good idea. Perhaps yes.

@ggarber
Copy link
Contributor Author

ggarber commented Apr 11, 2023

I'll be off for a few days. But as Jose said, please move to a separate file and rename the file and the class to FakeTimeHandle to be in sync with naming changes done in flatbuffers branch.

I don't really know if moving GetTimeXx() methods from Utils to Timer is a good idea. Perhaps yes.

Ok, I'll move it as soon as i have time (doing this one in my free time :)). Thx for taking a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants