Skip to content

Conversation

@guillaumemichel
Copy link
Collaborator

@guillaumemichel guillaumemichel commented Aug 28, 2025

Summary

Refactor tests to use github.com/coder/quartz instead of github.com/benbjohnson/clock or its fork github.com/filecoin-project/go-clock

Refactor tests to use testing/synctest instead of github.com/benbjohnson/clock, its fork github.com/filecoin-project/go-clock or github.com/coder/quartz.

See this blogpost

Benefits

  • Better control of time (and goroutines!) in tests
  • Deterministic testing (no flakes)
  • Faster
  • Standard library

@dennis-tra
Copy link
Contributor

A quick fly-by comment because I'm still being notified of all the PRs :D

What about using https://go.dev/blog/synctest instead of introducing another 3rd party lib? synctest is only out since Go 1.25 so I guess it cannot be introduced right away because of the go-libp2p-kad-dht backwards compatibility promise of the latest two Go versions? Or does such promise not exist (anymore)?

@guillaumemichel
Copy link
Collaborator Author

Thanks for the suggestion @dennis-tra, I found a way to run the synctest tests only for go1.25+.

Adapting tests from quartz to synctest isn't too involved. I already converted the connectivity tests to synctest and will proceed to use synctest instead of go-clock for the provider tests too.

The goal is to use synctest wherever needed, and not quartz nor go-clock.

@guillaumemichel guillaumemichel changed the title provider: use quartz for testing time provider: use synctest for testing time Aug 28, 2025
@guillaumemichel guillaumemichel marked this pull request as ready for review August 28, 2025 21:25
@guillaumemichel guillaumemichel requested a review from a team as a code owner August 28, 2025 21:25
@guillaumemichel guillaumemichel mentioned this pull request Aug 28, 2025
56 tasks
@MarcoPolo
Copy link
Contributor

+1 synctest. Technically not fully deterministic, but that's fine.

remember we have the ability to create simulated networks: https://github.com/libp2p/go-libp2p/tree/master/x/simlibp2p that work with synctest in case its helpful

@dennis-tra
Copy link
Contributor

dennis-tra commented Aug 29, 2025

Technically not fully deterministic, but that's fine.

it's not? do you have a link where I can read up on the details? I had read the linked recent blog post and a few other resources but must have missed that detail.

@MarcoPolo
Copy link
Contributor

MarcoPolo commented Aug 29, 2025 via email

@guillaumemichel guillaumemichel merged commit d3310b4 into provider-connectivity-mgmt Sep 4, 2025
9 checks passed
@guillaumemichel guillaumemichel deleted the provider-quartz branch September 4, 2025 13:07
@guillaumemichel guillaumemichel restored the provider-quartz branch September 4, 2025 13:13
guillaumemichel added a commit to guillaumemichel/go-libp2p-kad-dht that referenced this pull request Sep 17, 2025
guillaumemichel added a commit to guillaumemichel/go-libp2p-kad-dht that referenced this pull request Sep 17, 2025
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.

4 participants