jonboulle / clockwork

a fake clock for golang
Apache License 2.0
625 stars 57 forks source link

Document that this does not affect Context.WithTimeout #76

Closed sourcefrog closed 6 months ago

sourcefrog commented 8 months ago

Hi, thanks for the nice package.

I was trying to fix some flaky tests by converting them to use this. However, the tests use Context.WithTimeout and it looks like the fake clock, even if injected into a context, is not respected by that code?

I can imagine how that might be impossible to fix without core library changes but it might be nice to document it to save people time?

DPJacques commented 8 months ago

All true, and I agree that it is unlikely we'd be able to get the core libraries to respect this. We can add documentation to that effect.

even if injected into a context

Just adding a personal note that I think injecting it into a context is not something I'd endorse. It was added to this package, but doesn't mean it is a great pattern. Since this package is all about testing, I'd advise adding the fake clocks directly to your structs as unexported fields.

sourcefrog commented 8 months ago

Oh, good to know. Maybe that should be in the docs too, depending how much you want to editorialize?

I'll avoid it going forward, but it was just my attempt to get timeouts to work plus to deflake existing tests without major rewrites.

DPJacques commented 6 months ago

Added a warning on clockwork.AddToContext.