Consider a form that allows users to search for holidays. It is expected behaviour that the form remembers previous searches –  so if a user previously searched for “4 guests”, the form should pre-populate with “4 guests” the next time the page loads.

This is a simple enough idea, but an odd bug was raised for this component. Ten points to Gryffindor for spotting it before knowing the symptoms: click to see the code.

You read the code code, yes? And you spotted the bug? No? Either way a summary:

1) The components are split into ‘container’ and ‘presentational’. Here is why.
2) createContainer returns a component instance that had state from a flux store passed to it as props.
3) If state.enquiryForm.numberOfGuests is falsey then default to 1.
4) We have some tests that confirm logic is as expected (1 for falsey, 5 for otherwise).

The unit tests passed, and we were happy when we manually tested the component in the browser — the select box ‘remembered’ the value. Great. Deploy!

So why was there a bug? The report was “the number of guests select box adds 1!”

After checking the component and the code we concluded ‘nope’ — The component did not add one to the value — when we selected “one guest”, on reload we got “one guest” in the form. When we selected “five guests” we got “five guests” on reload. When we select “four guests” in then we get “five guests” on reload. Wait, what?

Investigating the bug

Yes, the bug was reproducible . Great. Below is a table showing ‘the value we selected in the form’ against ‘the value pre-populated on refresh’:

Every even number adds one to itself!? Okay…
Go back to the code and see if you can see the bug. No?

Bitwise OR, Logical OR

There’s a big difference between | and ||. Due to a simple typo a bitwise OR was used in lieu of a logical OR. The outcome of the missing pipe was this unexpected behavior.

A bitwise OR performs a logical OR on each bit of in the numeric value — so five will be converted to 0101, and one will be converted to 0001, and each bit will be OR’d — if either of the bits are 1 then use 1 in the return value’s position:

But when we use an even number such as four or one, we get:

and again with six or one:

but to labour the point with seven and one:

Lessons learned

‘Write more unit tests’ is not the answer to this problem. After all, unit tests were written and the bug still got through. We could write tests specifically targeting ‘bitwise OR’ bugs, but this sets precedence against all similar scenarios. This would take an inordinate amount of time. Moreover, unit tests are for confirming logic, not detecting bugs. This type of bug can be more effectively detected using other means.

This bug was missed by three developers during code review. Does that indicate a human problem with the review process? I think not. Sometimes humans see what they expect to see. With this in mind, as put by W Edwards Deming: 100% inspection is only 100% effective if the inspector detects 100% of defects 100% of the time…which is impossible due to human nature. After all, if humans were that precise there wouldn’t be any defects made in the first place.

We can make ourselves more precise though by using static analysis tools like eslint. We turned on no bitwise-operators rule and it shows a helpful warning message. It should not interfere with the day to day tasks  of the project— if an e-commerce site sees itself manually wrangling bits then it’s probably doing something it shouldn’t.