Testing best practices

Testing best practices

Add more best practices here!

Making tests more readable

it() descriptions should read like a sentence

it("value()", () => { ... }); // doesn't explain what's being tested
it("can get and set value()", () => { ... }); // good: reads like a sentence, explains what the test does

Assert error throwing should include the error message.

(<any> assert).throws(() => c.foo(), Error, "", "The reason"); // bad
(<any> assert).throws(() => c.foo(), Error, "Error message", "The reason"); // good

The reason for that is that if we are expecting an error, but we are getting a different error, we should catch that as unexpected behavior. The reason against this change was code fragility (if we change the error message we don't want to duplicate the code, but we already do that for any of the behavior change in code that reflects into tests)

Use assert.operator for greater-than or less-than checks

assert.isTrue(a > b); // does not produce a helpful error message
assert.operator(a, ">", b); // better

---

Checking the size of a D3 Selection

The "length" of a D3 Selection is always 1, regardless of how many elements are in it:

d3.select().length; // 1, not 0

The correct way to check the size of the Selection is to assert that its size() is equal to a particular value:

assert.strictEquals(selection.size(), expectedSize, message);

Iterating over a d3 selection

Use

whateverSelection.selectAll("line").each(function() {
  let lineSelection = d3.select(this); // "this" refers to the DOM node inside the loop
});

Instead of hacky stuff like

whateverSelection.selectAll("line")[0].forEach((line) => {
  let lineSelection = d3.select(line);
});

Always check the size of a d3 selection

let edges = dbl.content().selectAll("line"); // this selection
assert.strictEqual(edges.size(), 4, "the edges of a rectangle are drawn"); // check here
edges.each(function() { // otherwise asserts here do not get called
  ...
}

Pixel comparison / Float calculation checks

Use assert.closeTo in scenarios where we are checking pixels against data values and in float arithmetic.

Do not have svg.remove() inside an afterEach() block

That is because if something goes wrong with the test, we want to have the visual.

Minor

• Use plot.content() instead of plot._renderArea
• Make sure we use the it() semantically. It should read like a sentence. For example it("does not draw rectangles for invalid data points") instead of it("data points that are not valid, do not draw rectangles on the plot")

Suggestions:

These are just suggestions of best practices, slowly move them upwards as people agree. In add proposer & agreers to each to have a sense of what is okay

• Top level describes should act as category delimiters and should not have beforeEach(). That is because sometime we want to add a new, totally unrelated test to some category (say Plots.Line) and the beforeEach() does not make sense. Top Level means the title of the file (so for example "Plots" and "RectanglePlot" will be top level) (@acioara)