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

introduce an API to set a mock clock #20

Merged
merged 1 commit into from
Jul 9, 2022
Merged

Conversation

marten-seemann
Copy link
Contributor

@marten-seemann marten-seemann commented Jul 8, 2022

Fixes #17. Closes #18.

This should be significantly easier than #18. It only adds a SetClock function, and no way to switch out the clock. That means that in tests, you have to define a global clock variable (and not a separate clock for every test).

This also means that I had to create a new testing package to test the mock clock. At some point, we probably should rewrite the tests to take advantage of the mock clock, but this will be a larger undertaking, especially since registering a new meter is not done synchronously (it's put in the registerChannel, and picked up by the sweeper eventually), which makes it impossible to know when to advance the clock.
In https://github.com/libp2p/go-libp2p-core/pull/276/files#diff-3dda9486f00364eaac26b64ff16560cde7b6cf8ff03eda4de9752f0fb0a96f42R130, I work around this by sleeping for a short while.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Looks like a strict improvement, but we're going to need to migrate all the tests (and I'm not sure why we're moving the mock test to a new package.

meter.go Outdated Show resolved Hide resolved
sweeper.go Outdated Show resolved Hide resolved
sweeper.go Outdated Show resolved Hide resolved
@marten-seemann
Copy link
Contributor Author

@Stebalien I was updating the PR description, which should answer your question.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM.

@marten-seemann marten-seemann merged commit 3f13d05 into master Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a mock clock
2 participants