Hacker News new | past | comments | ask | show | jobs | submit login

Dear software engineers working with money or crypto: please write some tests.



More importantly, if you're dealing with anything critical, start with the assumption your code if full of bugs and will fail in ways you haven't even thought of. Now make sure you have fail-safes and procedures in place outside of your code to catch these failures and deal with them as early as possible.


In some ways this is worse than writing code for something like the Therac-25: there are hostile people out there who will actively try to destroy you. If they can cause you $10 million in damages to get $10,000 for themselves they will consider that a good day.

This was the guy who wrote his own SSH server (in PHP, FWIW) and put it into production, right? This whole thing is a disaster waiting to happen.


>This was the guy who...

I missed something. Which guy?



Maybe MtGox does have tests but they wrote their own unit testing framework and it doesn't unit test correctly?

http://blog.magicaltux.net/2009/09/19/striving-for-a-better-...


Go kind of makes you do this. Last weekend I was writing some code, ran the tests, and saw that they failed. I debugged my test harness for a while, only to find that the bug was actually in my real code. (Overall I kind of like the strategy, I'd much rather debug my own for-loop-over-test-data than someone else's. But it does lead the mind down different paths than when you use something like JUnit/Hamcrest.)

Incidentally, this is why "test first" is more than just a methodology for selling high-priced consultants. At least it lets you see your tests fail and then pass, rather than just pass. Lots of common patterns pass in the presence of incorrect code.

An example that a coworker was complaining to me about recently:

  void testFooBarException() {
    try {
      thisShouldThrowFooBarException()
    } catch (FooBarException ignored) {}
  }
Can you spot the bug? The test still passes even if thisShouldThrowFooBarException doesn't throw an exception. Oops.

I personally avoid this by checking that I can make the test fail when I expect it to fail, by editing some values or commenting something out. But that doesn't scale, that only saves you once.

Something to think about.


In case anyone is wondering, that code should read something like:

  void testFooBarException() {
    try {
      thisShouldThrowFooBarException()
      fail() // Should have thrown exception
    } catch (FooBarException ignored) {}
  }
If you're feeling super fancy, you can even do some asserts in the catch block to make sure the FooBarException has an expected exception message.


I'm glad I work in a language where this is

  assertRaises(FooBarException, thisShouldThrowFooBarException)
Impossible to mistype, and states exactly what should happen.


First thing to do when working with the former is to write some code that will allow you to write the latter.


With Junit, just write

  @Test(expected = FooBarException.class)
  void testFooBarException() {
      thisShouldThrowFooBarException()
  }


PHPUnit has this in the comment annotations

    /**
     * Tests that thisShouldThrowFooBarException throws FooBarException
     *
     *  @expectedException FooBarException
     */
    public function testFooBarException() {
      thisShouldThrowFooBarException();
    }
but I much prefer

    /**
     * Tests that thisShouldThrowFooBarException throws FooBarException
     */
    public function testFooBarException() {
      $this->setExpectedException('FooBarException');
      thisShouldThrowFooBarException();
    }
Much more explicit and you're less likely to miss it when trying to grok someone else's unittest.


That's quite an old fashioned way of testing exceptions, TestNG has allowed you to do the following since 2005:

    @Test(expectedExceptions = FooBarException.class)
    public void test() {
      thisShouldThrow();
    }
If the method doesn't throw or throws the wrong kind of exception, this test will fail.


That has the problem of separating the statement that is supposed to throw and then thrown exception. E.g. a slightly more complicated test case:

    @Test(expectedExceptions = FooException.class)
    public void test() {
      firstDoThis();
      thenThat();
      thenSomeMore();
      thisShouldThrow();
    }
Now you're only asserting that any of these statements throw, anywhere in the code under test. That's significantly weaker and I've seen it mask real problems in code.

I think using plain try something/fail/catch is clearer, or using Closures and an assertThrows if your language supports it.


You should probably think of splitting up your test case if it's this complicated.


You can always revert to the old try/catch/fail if you need to test something more complex (e.g. testing the message of the exception) but most of exception tests fall in the simple case shown above.


In that case, you first write a test case ensuring that the first three statements won't throw.


JUnit4 kind of helps with ExpectedExceptions. try-catch doesn't usually have a good reason to exist in unit tests.


> avoid this by checking that I can make the test fail when I expect it to fail, by editing some values or commenting something out.

Whenever I do this, I feel awful, because:

> But that doesn't scale, that only saves you once

You need to fuzz test your tests. (I always forget the name of the awesome Java tool for this....) If you can randomly mutate your test code (negate boolean conditional, etc) and your tests still pass, they fail.


I often deliberately break something and make sure it fails before accepting that my test actually tests for what I think it does. Not perfect, but it gets you some of the way there.


Check out a thing called "mutation testing" - it changes your code and checks for test failures. If no tests fail, the mutation test fails.


oh really? you edit your some values or comment something to make sure that your tests fail on failure?

shouldn't you be automating that? :-D


MtGox: the apotheosis of Not-Invented-Here.

EDIT: NIH is pretty bad overall, but it's especially terrible in crypto, where 'useless' and 'useful' take on something akin to binary values, rather than a continuum.


Remember, when working with bitcoin, that transactions are _irreversible_. Even in the real billion-dollar finance world transactions have been reversed for serious software errors. Not in bitcoin.


Dear software engineers working with money or crypto: hire security consultants. Pay them with your favorite cryptocurrency.



And then test them live on the testnet. That's what it's there for.




Join us for AI Startup School this June 16-17 in San Francisco!

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: