kevinweil/elephant-bird — Issue #146: TestInvoker#testSpeed() may fail on slow machines
Author: miguno
Created: 2012-02-22 16:19:37+00:00
State: open

Hi there,

we run into an issue where the following test in ```src/test/com/twitter/elephantbird/pig/piggybank/TestInvoker.java``` may fail on slow(er) machines. In our case, for instance, this happens on our build server. On my local dev box, the test sometimes passes and sometimes it doesn't (hmm).

```
@Test
public void testSpeed() throws IOException, SecurityException, ClassNotFoundException, NoSuchMethodException {
EvalFunc log = new Log();
Tuple tup = tf_.newTuple(1);
long start = System.currentTimeMillis();
for (int i=0; i < 1000000; i++) {
tup.set(0, (double) i);
log.exec(tup);
}
long staticSpeed = (System.currentTimeMillis()-start);
start = System.currentTimeMillis();
log = new InvokeForDouble("java.lang.Math.log", "Double", "static");
for (int i=0; i < 1000000; i++) {
tup.set(0, (double) i);
log.exec(tup);
}
long dynamicSpeed = System.currentTimeMillis()-start;
System.err.println("Dynamic to static ratio: "+((float) dynamicSpeed)/staticSpeed);
assertTrue( ((float) dynamicSpeed)/staticSpeed < 5);
}
```

The "problem" with this test is that its success seems to depend on the build environment. Even though a relative ratio is used, this alone apparently does not guarantee that the test completes successfully on _any_ box that the build is being run on. Also, the test is not a functional test but a performance test.

Hence I'd say that one can argue pro and con whether it should be a test that a) is enabled by default and/or b) whether failing this test should make the full build fail (maybe a warning would be more appropriate?).

On a related note: Where does the value/number of ```5``` (last line in ```testSpeed()``` ) come from? Was it arbitrarily chosen? FWIW, on my dev box most dynamic-to-static ratios are in the 4.9-5.3 range, and on our build server they are in the 6.1-6.5 range. At the moment, we have decided to ```@Ignore``` this test in our build for the time being.