Your test suite is trying to tell you something(blog.jgc.org)
blog.jgc.org
Your test suite is trying to tell you something
http://blog.jgc.org/2013/07/your-test-suite-is-trying-to-tell-you.html
43 comments
The real problem is `duration * time.Millisecond` makes no sense. The time api seems a bit obtuse. Why not `duration.milliseconds`?
Agree. Part of the problem here is that the Go time.Duration is just an int64 so you can do arithmetic on them. It would be better if there was something like NewDurationMilliseconds(). Thus the type here allowed me to shoot myself in the foot.
Having said that it doesn't change the fact that looking into the 'flaky' test was the right thing to do. Even if the type system were different I'm sure I would have shot myself in the foot in some other way.
Having said that it doesn't change the fact that looking into the 'flaky' test was the right thing to do. Even if the type system were different I'm sure I would have shot myself in the foot in some other way.
Seems it would rather be better if there was a .net TimeSpan / JodaTime Duration type, or better yet, a type system that understood units.
time.Duration is a count of nanoseconds, and time.Millisecond is a constant that represents the number of nanoseconds in a millisecond.
http://golang.org/pkg/time/#Duration
http://golang.org/pkg/time/#Duration
This is the sort of thing having a Haskell-like typesystem would fix - multiplying something by "time.Millisecond" is difficult to discern the semantic meaning of, especially when nothing in the code/language prevents you from doing it multiple times.
The idea of storing this information as just a regular Int64 bothers me after spending so much time in type system nirvana.
The idea of storing this information as just a regular Int64 bothers me after spending so much time in type system nirvana.
> The idea of storing this information as just a regular Int64 bothers me after spending so much time in type system nirvana.
Its not a regular int64, its a distinct named type (Duration) where int64 is the underyling time. I'm still getting familiar with Go -- and don't have it on the machine I'm using at the moment -- but from the docs named types are incompatible with other named types with the same underlying types in numerous contexts, but it looks like one place where named types are leaky abstractions is their interaction with operators.
Its not a regular int64, its a distinct named type (Duration) where int64 is the underyling time. I'm still getting familiar with Go -- and don't have it on the machine I'm using at the moment -- but from the docs named types are incompatible with other named types with the same underlying types in numerous contexts, but it looks like one place where named types are leaky abstractions is their interaction with operators.
To illustrate this point, this code:
package main
import (
"time"
"fmt"
)
func ident(x int64) int64 {
return x
}
func main() {
t := time.Duration(1)
v := math.Inf(t)
}
cannot use t (type time.Duration) as type int64 in function argument
[deleted]
"By me." - I've been there many times!
I hate that SCM systems commonly describe who made changes as "blame" (couldn't it be "praise" for when you want to find out who wrote exceptional code?) and yet it's wonderful to be able to look back into history and see "who" was responsible for a block of code. It can be a real learning experience!
I hate that SCM systems commonly describe who made changes as "blame" (couldn't it be "praise" for when you want to find out who wrote exceptional code?) and yet it's wonderful to be able to look back into history and see "who" was responsible for a block of code. It can be a real learning experience!
'svn praise' is an alias for 'svn blame' (or the other way round). The more neutral wording is usually 'annotate'. To be honest, I have rarely used these commands to check who had written such a beautiful code...
Someone once said that the only real measure of code quality was the number of WTFs/minute emitted while reading it:
http://www.osnews.com/story/19266/WTFs_m
http://www.osnews.com/story/19266/WTFs_m
Might be from Joel Spolsky/Bill Gates:
"...a person who came along from my team whose whole job during the meeting was to keep an accurate count of how many times Bill said the F word. The lower the f*-count, the better."
http://www.joelonsoftware.com/items/2006/06/16.html
"...a person who came along from my team whose whole job during the meeting was to keep an accurate count of how many times Bill said the F word. The lower the f*-count, the better."
http://www.joelonsoftware.com/items/2006/06/16.html
bazaar uses the blame/praise/annotate trio as well.
It's only blame because on the 90th time you've found a rotting turd in your 1mloc enterprise codebase that nearly shot the company properly, you really do want to blame it on that budget outsourcing outfit or the HR member who recruited that junior dev without even vetting them :)
(this is what I unfortunately sit right in the middle of)
(this is what I unfortunately sit right in the middle of)
You know many sites these days have various badges that you earn. I propose the addition of a badge that you admit to---the 'By Me' badge...
Without knowing more about the source, it's hard to say for sure, but would an explicit test for a failure to connect to memcached have uncovered this? That is, if there is common code to connect to memcached, and a test to purposely generate a timeout, it seems like it would have jumped out as taking far too long.
If there's no common code to connect, and every user of it has its own implementation, there might be more bugs like this lurking. Every user of memcache in this particular code base would need to have its own "what if memcached is slow or down?" type test. It seems such a test did not already exist.
Following the events in this post, is there now a test to explicitly make sure the delays are reasonable, or was the extra multiplication just removed?
If there's no common code to connect, and every user of it has its own implementation, there might be more bugs like this lurking. Every user of memcache in this particular code base would need to have its own "what if memcached is slow or down?" type test. It seems such a test did not already exist.
Following the events in this post, is there now a test to explicitly make sure the delays are reasonable, or was the extra multiplication just removed?
The big change was to write a mock memcached server where I could control everything. The extra multiplication was removed and a bunch of new tests written using the mock server.
Ah, this reminds me to go use the new std::chrono classes to strongly type my time unit variables.
If you listen to the pain or failings of your test suite, you will likely end up with better code. The problem is most developers when testing gets hard don't look at their code and go "why does my code make this so difficult?" they wil say "testing sucks" and delete their tests.
It's amazing what you will learn if you pay attention to the pain.
It's amazing what you will learn if you pay attention to the pain.
I agree. This example is a perfect candidate to further abstract the source to remove ambiguities and better test the expectations of the source.
Great anecdote. I've never really agreed with having massive system-level automated tests, because more often than not the random failures like this do just get ignored.
I guess it depends on the culture within your team, how much you all care about your test suites.
I guess it depends on the culture within your team, how much you all care about your test suites.
It amazes how often I see otherwise competent engineers ignore spurious failures – they don't even open a bug report!
Software doesn't fail spuriously – there's always a cause.
Software doesn't fail spuriously – there's always a cause.
Interesting analysis, but yup, that's what tests are for! If you've got a sporadically failing test somewhere, that's probably sign of a bug.
The problem is that 95% of the time, such failures are a bug in the test or test environment, not in the code. Spending two days chasing down a bug in your test feels a lot like wasted effort. Once you've done it 19 times you're very tempted to just disable the test and stop wasting time. This blog post is a good reminder that sometimes the bug is actually in the code: persevere.
The question is what's the cost/benefit on fixing such things? Not everyone is in a position to stop working on features for two days to track down something like this.
This particular bug is also a strong argument for using more advanced type systems, even if you have good test coverage.
This particular bug is also a strong argument for using more advanced type systems, even if you have good test coverage.
For something like this, you don't need an advanced type system. Even C can do
struct duration { int in_nanoseconds; };
struct duration seconds_duration (int in_seconds) {
struct duration dt;
dt.in_nanoseconds = in_seconds * 1000000000;
return dt;
}
/*TODO: milliseconds_duration(), microseconds_duration(), etc */I highly recommend doing this kind of thing, incidentally. Just because two things have the same backing representation does not mean they're of the same type, and wrapping in a struct tells the compiler to keep them separate. It doesn't have any impact on performance; C structs don't carry anything more than their member and you've only got the one member. I do this with indexes and ids as well.
One would hope that 'typedef' would define a type in C, protecting the programmer from silent incorrect casts, but of course it doesn't, because C hates programmers.
Well, typedef "defines a type", but (unfortunately) in a purely syntactic sense. If you're using typedef for something that's not wrapped in a struct or union, think hard about whether you mightn't want to also do that.
This particular bug is also a strong argument for using more advanced type systems, even if you have good test coverage.
Yes, it's a great pity that the compiler didn't say "You're multiplying a time.Duration by a time.Duration and expecting it to be a time.Duration".
Yes, it's a great pity that the compiler didn't say "You're multiplying a time.Duration by a time.Duration and expecting it to be a time.Duration".
I'm not sure whether you're serious or not (and I see a few different jokes you could be getting at otherwise), but multiplying time by time should give you time squared, not time again. Unfortunately, even Haskell doesn't catch errors of that type if you want to use the ordinary mathematical operators, because (*) :: Num a => a -> a -> a.
In this case, that's not what's going on, though. It's multiplying time by unitless scaling factor, which doesn't present the same problem. However, there's a few ways you can get at the issue with the help of the language, you just have to distinguish between the different units you might be using to represent time - this doesn't really take an advanced type system: the example of having a .milliseconds method is along these lines.
In this case, that's not what's going on, though. It's multiplying time by unitless scaling factor, which doesn't present the same problem. However, there's a few ways you can get at the issue with the help of the language, you just have to distinguish between the different units you might be using to represent time - this doesn't really take an advanced type system: the example of having a .milliseconds method is along these lines.
Have you seen the dimensional library? It's a slightly abusive use of the type system, but will check units at compile time. See e.g. http://code.google.com/p/dimensional/wiki/GMExample
I think so; I've seen similar things, for sure. Obviously Haskell does have the ability to make this distinction; just (unfortunately) it's somewhat incompatible with the standard prelude, and thus common practice.
The F# compiler does that.
For those that are interested, here's a link to the docs: http://msdn.microsoft.com/en-us/library/dd233243.aspx
Or an indicator of a non-deterministic test. Testing date manipulations with any "now" values can lead to fun like this. Add in timezones and the tests will pass in the US and fail in Japan.
After using jaycfields' "expectations" Clojure testing library, I've become very very appreciative of libraries that expose an easy way to freeze time, for exactly the issues you described.
Makes me want to go zap a long standing ocassional test fail I got.
It seems that sometimes a netty hashwheeltimer is executing a timeout you add immediately, instead of after 2 seconds. And its the simplest code, and the 2 seconds we are trying to set are literals, so it ready surprises me. Find it hard to imagine a bug in netty and it always affects the same test, despite all our requests setting an aggressive 2 second timeout...
It seems that sometimes a netty hashwheeltimer is executing a timeout you add immediately, instead of after 2 seconds. And its the simplest code, and the 2 seconds we are trying to set are literals, so it ready surprises me. Find it hard to imagine a bug in netty and it always affects the same test, despite all our requests setting an aggressive 2 second timeout...
Why do they call it a "memcached" server rather than a "memcache" server? I realize that the daemon process for a server application is usually the normal name followed by a 'd', but that is not usually how we refer to the server or application. For instance, we say "mysql" server, not the "mysqld" server, even though that is the name of the long running process.
Because that's how memcached brands itself.
http://memcached.org/ -- the URL has a d (unlike mysql)
"What is Memcached?" -- it's how they talk about themselves.
It's even in the logo...
http://memcached.org/ -- the URL has a d (unlike mysql)
"What is Memcached?" -- it's how they talk about themselves.
It's even in the logo...
I think this is a really good example of where languages should have built in support for units. I was just having a look around and found the units module for python (https://pypi.python.org/pypi/units/). I really like the example at the end where they have modified pypy for a native unit syntax.
I think built is support for units is a bit to specific. Support for cheap type alias'es would work, and be far more general. What I mean is something like:
type Inch = int type Centimeter = int
And then have the compiler enforce these as different types (as opposed to alias'es), so if you want to use an Inch, where the function decalares it expects a Centimeter, you would be force to cast it (or have the compiler auto cast using your conversion function).
This doesn't get you support for multidimensional units (IE, define a function that goes from arbitary unit u to u^2), which would require a much more complicated addition to the type system.
type Inch = int type Centimeter = int
And then have the compiler enforce these as different types (as opposed to alias'es), so if you want to use an Inch, where the function decalares it expects a Centimeter, you would be force to cast it (or have the compiler auto cast using your conversion function).
This doesn't get you support for multidimensional units (IE, define a function that goes from arbitary unit u to u^2), which would require a much more complicated addition to the type system.
> I think built is support for units is a bit to specific. Support for cheap type alias'es would work, and be far more general
Go has support for cheap type aliases for which names typed with the same underlying type are distinct types which are not mutually assignable, and, in fact, Duration is such a type with int64 as the underlying type.
Go has support for cheap type aliases for which names typed with the same underlying type are distinct types which are not mutually assignable, and, in fact, Duration is such a type with int64 as the underlying type.