Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

use a mock clock in bandwidth counter tests #267

Closed
marten-seemann opened this issue Jun 24, 2022 · 4 comments · Fixed by #276
Closed

use a mock clock in bandwidth counter tests #267

marten-seemann opened this issue Jun 24, 2022 · 4 comments · Fixed by #276
Labels
effort/hours Estimated to take one or several hours exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P1 High: Likely tackled by core team if no one steps up

Comments

@marten-seemann
Copy link
Contributor

These tests are currently flaky, and we won't be able to proceed with repo consolidation (libp2p/go-libp2p#1556) until they're fixed. Injecting a mock clock (similar to what we did in https://github.com/libp2p/go-libp2p/pull/1516/files) should be a straightforward way to test this reliably, and dramatically speed up the test at the same time.

@marten-seemann marten-seemann added help wanted Seeking public contribution on this issue good first issue Good issue for new contributors P1 High: Likely tackled by core team if no one steps up exp/beginner Can be confidently tackled by newcomers effort/hours Estimated to take one or several hours labels Jun 24, 2022
@schomatis
Copy link

schomatis commented Jun 27, 2022

@marten-seemann I suspect I'm missing some context here (I'm new in libp2p and the term 'bandwidth counter' doesn't mean anything to me) and will need this laid out in a bit more detail:

These tests are currently flaky

Which tests?

Injecting a mock clock

Where?

@marten-seemann
Copy link
Contributor Author

Which tests?

The bandwidth tests.

Where?

Into the metrics/BandwidthCounter. It’s using the flow metrics repo, so probably we need to do it there too. Basically, the goal is that we don’t use any calls to time.Now() any more, but only to a clock.Now(). When run in production, clock.Now() use time.Now(), but in tests we can use a mock clock.

@schomatis
Copy link

Sounds good, working on this.

@schomatis
Copy link

Upstream work in libp2p/go-flow-metrics#18. After that lands will proceed with applying the mock clock here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort/hours Estimated to take one or several hours exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants