Comment by jerf | Hacker News Reader

Comment by jerf | Hacker News Reader

Parent Full thread

Mocking out the time functions means you don't get any race conditions. If you try to just twiddle with how long the time functions run, you still never quite know how long to set them. Bear in mind that even rather generous timeouts like a full millisecond to set a map entry in another goroutine can still fail if your system's CPU is loaded or the system is in swap, and spurious test failures are spurious test failures regardless of their cause. Often with this sort of code you're testing in one goroutine and the test code is in one or more other goroutines. If your test code can emit something down a synchronous channel, then your test code can also be sure it is staying in sync. I mean, even to test with "extra special small test timeouts" still means you're writing in extra test structure, you might as well just use one of these time mocking things. (In fact, you should generally always use time mocking libraries. In all languages, direct access to the system clock that can not be easily redirected for testing is at least a code smell and could possibly even rise to the level of "antipattern".)

I have a lot of places in my Go test code where my test code is deliberately pushing things down channels. It's one of the top reasons my test code often still ends up being in the same package, so it gets private access to those channels for safe, properly-sync'ed testing. (Not that I'm really all that concerned about trying to test only the public interface; I don't have a lot of troubles with that anyhow. YMMV.) I also have some places in my code where I have channels in the private interface of some goroutine server whose sole purpose in life is to sync with the tests. This generally appears when I have some server that I am sending a message that I expect to change the state of the server, but for which there is no reply. In order to verify that the proper changes have occurred in the data structures of the server, I need to sync with the change before I do the check. Having a simple struct{} channel whose sole purpose in life is to synchronize works well enough for that.

> Mocking out the time functions means you don't get any race conditions.

This is a common misapprehension. Actually, even if you fully mock out time, you can still get race conditions, because goroutines can remain active regardless of the state of the clock, and there's no general way to wait until all goroutines are quiescent waiting on the clock. This is not just a theoretical concern - this kind of problem is not uncommon in practice.

I think clock-mocking can be very useful for testing hard-to-reach places in leaf packages. But at a higher level, I think it can end up producing extremely fragile tests that depend intimately on implementation details of packages that the tests should not be concerned about at all. In these cases, I've come to prefer configuring short time intervals and polling for desired state as being the lesser of two evils.

OK, correction acknowledge (no sarcasm), mocking out time functions means you can write test code that doesn't have any race conditions.

"and there's no general way to wait until all goroutines are quiescent waiting on the clock."

Hence my semi-frequent usage of "sync" channels which I described in the previous post.

"But at a higher level, I think it can end up producing extremely fragile tests that depend intimately on implementation details of packages that the tests should not be concerned about at all."

I'd rather have a test that correctly reasonably verifies that a package is correct (or at least "passes the race detector consistently") and reaches into some of the private details than fail to test a package. Too many bugs I've found that way.

It may also help to understand my opinion when I point out that I tend to break my packages down significantly more granularly than a lot of the rest of the Go community, which in my opinion is a little too comfortable having the "main app" directory contain many dozens of .go files. My packages end up way smaller, which also mitigates against the issues of excessively-coupled tests. I have a (not publically published) web framework, for instance, that is broadly speaking less featureful than some of the Big Names that are all in one directory (though it has some unique ones all its own), but is already broken up into 16 modules.

> I'd rather have a test that correctly reasonably verifies that a package is correct (or at least "passes the race detector consistently") and reaches into some of the private details than fail to test a package. Too many bugs I've found that way.

I agree with this, with the caveat that if you can test a package with regard to its public API only, it is desirable to do so because it gives much greater peace of mind when doing significant refactoring.

The difficulty comes in larger software where the package you're testing uses other packages as part of its implementation which also have their own time-based logic. Do we export all those synchronisation points so that importers can use them to help their tests too? If we do, then suddenly our API surface is significantly larger and more fragile - what would have been an internal fix can become a breaking change for many importers.

Reply on news.ycombinator.com