Symphonious

One of the major challenges in my job is testing our product. Now most people think that testing is reasonably easy but requires discipline, this is not true if the product you write happens to be a styled text editor and it’s nearly impossible to do really well if you’re working with something as flexibly defined as HTML.

The problems start at the unit testing level. Try taking the standard JTextPane class and writing unit tests for it. How do you test that it can render a HTML list correctly? You could write a test to make sure that the list numbering at least comes out in order and in the right format but that still won’t guarantee that the list numbers actually paint correctly. For instance, we recently had a bug where the list numbers painted correctly if the list fit on screen, but if it required scrolling, whatever item was at the top of the screen was numbered 1 even if it was actually the third item in the list. Our list numbering unit tests had no way of picking up on that because it was the actual rendering code (which we didn’t write) the was wrong.

So you may be thinking that integration tests should have picked up that bug. Not a chance. How do you write a set of tests for the list rendering that doesn’t break because we added a slight padding to the body which caused the list to indent a little more than it used to (but still render correctly).

The number of false negatives you get from automated tests is staggering and makes the tests completely worthless because adding new features or fixing bugs (like getting padding to work correctly) causes them to fail.

So what about manual testing? It’s not particularly effective either. Certainly it would (and did) pick up the list numbering bug I mentioned, however it didn’t pick up the other list bug we’ve had reported recently. To reproduce:

1. Start with a document that contains just an ordered list with 4 items.
2. Select and indent items 2-4.
3. Select item 4 and outdent it again.
4. Select item 3 and outdent it again.
5. Delete item 3.

Now tell me, when was the last time you saw a testing plan that would have included something like that? That’s certainly not the strangest bug I’ve seen either. The trouble is that every action that’s performed in the editor modifies the document state and that modification then affects every other operation that occurs in the document. There’s simply no way to isolate the operations on the document completely short of wiping the users document to get a clean document all the time (which doesn’t exactly make for a useful editor).

Lets take a look at something relatively simple - inserting a HR. Here’s what I’d go through to perform basic testing:

1. Insert a HR into a blank document.
2. Insert a HR at the end of the first and only paragraph in the document.
3. Insert a HR at the end of the first of two paragraphs in the document.
4. Insert a HR in the middle of a paragraph.
5. Insert a HR at the end of the first and only item of a list.
6. Insert a HR at the end of the first of two items of a list.
7. Insert a HR at the start of the first and only item of a list.
8. Insert a HR in the middle of a list item.
9. Insert a HR into an empty list item.
10. Insert a HR into an empty table cell.
11. Insert a HR at the end of a paragraph in a table cell.
12. Insert a HR at the start of a paragraph in a table cell.
13. Insert a HR in the middle of a paragraph in a table cell.
14. Insert a HR into an empty list item in a table cell.
15. Insert a HR at the start of a paragraph in a list in a table cell.
16. Insert a HR at the end of a paragraph in a list in a table cell.
17. Insert a HR in the middle of a paragraph in a list in a table cell.
18. Insert a HR into an empty table cell inside an empty table cell.
19. Insert a HR at the start of a paragraph in a table cell in a table cell.
20. Insert a HR at the end of a paragraph in a table cell in a table cell.
21. Insert a HR in the middle of a paragraph in a table cell in a table cell.
22. Insert a HR without first clicking in the editor to position the caret.
23. Insert two HRs in a row.

23 tests for inserting a HR, seems reasonably comprehensive right? Nope, if you inserted three HRs in a row it deleted the document (undo would recover it though). We also haven’t taken into account inserting into arabic text and making sure the text direction remains correct. Now think of how many tests you’d need to cover inserting a table and all the different things you can configure about a table (number of rows and columns, height and width as pixels or percentages, background colors, border size, alignment etc etc). Then think about lists, images, bold, italic, underline, strike through, sup, sub, merging cells, splitting cells, deleting things, selections spanning multiple blocks and a ton of other features. Then think about trying to parse and correct random HTML that’s thrown at you.

Considering all of that, how do you manage to quickly fix issues reported by customers and send them a new build that’s been well tested? How about when you have hundreds of new customers going into production each month? Did I mention you have an engineering team of 4?

The fact is, this stuff is incredibly hard. There’s a good reason that I laugh at people who think they can just whip up a HTML editor cheaper than our licensing fees, we’ve worked through all these issues - found them, fixed them and learnt how to engineer our code so that we’re confident they work without testing them. Testing is critical obviously, but reducing the number of errors in the code originally is even more important.

Trying to salvage some real point to this post: testing tools for text are practically non-existent and I’m dying for a way to actually automate these tests. If you’re up for a challenge, maybe testing text would be a good one to try.

Andrae Muys provides some excellent advice on implementing Object.equals() however I do have to correct one thing. Andrae presents the code:

class A {
  int n1;
  public boolean equals(Object o) {
    if (!(o instanceof A)) {
      return false;
    } else {
      A rhs = (A)o;
      return this.n1 == rhs.n1;
    }
  }
}

and suggests that it is incorrect. It is not. This is absolutely, 100% the correct way to implement equals for that class. The alternative he presents on the other hand is incorrect:

class A {
  int n1;
  public boolean equals(Object o) {
    if (o == this) {
      return true;
    } else if (o == null || !getClass().equals(o.getClass())) {
      return false;
    } else {
      A rhs = (A)o;
      return this.n1 == rhs.n1;
    }
  }
}

The problem with this second form only appears when a subclass is used:

class C extends A {
  public getValue() {
    return n1;
  }
}

With the first implementation, equals returns the correct value regardless of whether the object is an instance of C or A, but the second will not compare A and C correctly even though C has not changed the semantics of equality in any way. Andrae presents the following class to justify his assertion of the first way being wrong:

class B extends A {
  int n2;
  public boolean equals(Object o) {
    if (!(o instanceof B)) {
      return false;
    } else {
      B rhs = (B)o;
      return this.n1 == rhs.n1 && this.n2 == rhs.n2;
    }
  }
}

There is a bug here but it is in class B and not in class A. Class B is incorrectly assuming that it knows how to compare class A’s data, and more importantly class B is changing the behavior of A in a way that is not compatible with the original model. Essentially, class A defines equality as having the same n1 values and class B is in the wrong for changing that.

The golden rule here is that when you extend a class you are responsible for maintaining compatibility with the parent class. You should never extend a class and change the behavior of a method such that it no longer meets the contract of the original. This does not mean you can’t change the behavior, merely that it must be match the documented behavior - ie: it should do what the JavaDocs say the method does but not necessarily what the code actually does.

To give an example, consider the common example of a payroll system’s employee class:

public class Employee {

  /** The salary (in cents per year) for this employee. */
  private int salary;

  // Getter/setter for salary as appropriate and an appropriate constructor.

  /** Calculates the amount due to the employee for the current pay cycle.
    *
    * @return the amount (in cents) due to the employee for the current pay cycle.
    */
  public int calculatePayDue() {
    return salary / 52;
  }
}

It would be valid to extend the employee class as follows:

public class SalesPerson extends Employee {

    /** The amount of commission (in cents) due to this employee for the current
      * pay period.
      */
    private int commission;

    // Getters and setters as appropriate and an appropriate constructor.

    /** @see A.calculatePayDue() */
    public int calculatePayDue() {
        return super.calculatePayDue() + commission;
    }
}

The subclass changes the behavior of calculatePayDue but it still meets the original contract for the method, so it is correct. Similarly, Andrae’s class A meets the original contract for Object.equals and is thus correct, it is the implementation of equals in Andrae’s class B that breaks that contract and is thus incorrect.

There are two important lessons to learn from this (on top of the numerous important lessons to learn from Andrae’s post):

1. The JavaDoc is more important than the code. It is the JavaDoc that defines the contract for the method in Java and if the code doesn’t match the JavaDoc, the code is incorrect - though it may be more desirable to fix that by changing the JavaDoc if it’s a private method.
2. You should never extend a class in a way that breaks the contract of the super class. You should be able to run the unit tests for class A over class B and have most of them pass.

To clarify that second point, some of the tests for class A will fail because they actually test the behavior of A instead of the contract for A. The way to avoid this is to separate out the contract into an interface, you would then expect all the tests for class A’s implementation of the interface to also pass when an instance of class B was used (regardless of whether or not B extended from A or just implemented the same interface).

I was fortunate enough to be invited out to a friends place to see the fireworks last night. They happen to own the penthouse apartment on the river front with an awesome view of nearly all the fireworks. The fireworks are launched from numerous sites along the river so being able to see all of them is really quite unusual. Better yet though, one of the launching barges is positioned directly in front on the balcony as if it were a private show just for us.

That particular barge this year had some issues launching it’s fireworks. There were about three sections where all the other barges launched fireworks in unison but “our barge” sat there doing nothing. Apparently it was unplanned because at the end our barge suddenly started firing off every firework it had missed all at once. A display that was intended to take about 10 minutes was sent up in about a minute flat. Extremely impressive!

Also impressive was the dump and burn which was close enough to feel the heat hit you and felt like you could just reach out and catch the plane. I’ve never been overly impressed by fly-bys when standing at ground level at South Bank but from the penthouse it really is quite an experience.

Oh, and Iain has photos.

I’m currently wearing one of the shirts that James Gosling hurled into the crowd by various means which depicts Duke aiming a rocket launcher at a weird looking demon with four arms labeled complexity. Earlier today I was accosted by a very young man who asked what that was on my shirt. When I pointed to Duke and said “this guy’s called Duke” the response was: “he’s crap. I like this guy ’cause he has four arms”.

I’m not sure that’s the reaction the designer was after….

And they did.

Went to see We Will Rock You - the musical by Queen and Ben Elton last night and it was sensational. The songs fit into the story line brilliantly and the story itself was interesting and not just an excuse for singing the songs. The constant use of song references as bad puns just added to the experience for me and it was particularly impressive to see the customization for the Australian audience. The lead bohemian was titled after the great rock singer from the past “John Farnam” and when captured he was told - “This really is the last time”. That’s the kind of joke that flowed through the night and all of them went down extremely well.

The music itself was very loud and very energetic. Unless you are an absolute die-hard, noone can match Freddy Mercury type of person you’ll appreciate the vocal talent that performed the extremely difficult songs Queen put together.

Definitely worth seeing.