DEV Community

Sergey Sviridenko
Sergey Sviridenko

Posted on

100% line coverage did not catch the one bug that mattered

Symfony shipped a TUI component in 8.1 — a widget tree, a layout engine and a renderer for building full-screen terminal applications in PHP.

One thing up front, because someone will ask: php-tui/php-tui has been around much longer, it is mature, and it has around ninety times the downloads on Packagist. I picked symfony/tui because the project was already on Symfony and pulling in a second framework for one console command was not worth it.

On the ninth of August, when I started, symfony/tui had zero dependent packages on Packagist. There was no documentation page either — that one is still only two open pull requests.

I wanted a table: something that scrolls, sorts and filters over an array of rows. So I wrote one. The package is not the interesting part of this story. The interesting part is what building it taught me about my own testing.

The bug my test suite could not see

By the time I tagged 0.1.0, the widget had 92 tests and full line coverage of src/. PHPStan at level max, no baseline. I felt good about it.

Then I put the thing into a real command in one of my own projects, opened it in a small terminal window, and the header vanished off the top of the screen.

The widget never read the height it was given. It returned its configured number of rows whatever the terminal was: at stty rows 10 it produced 17 lines — a header, fifteen rows and a scroll indicator — into a window with room for ten. The layout engine pads short output but never trims long output, so the extra lines pushed the top of the frame out of view.

Here is why no test caught it. Every render test I had written looked like this:

$lines = $table->render(new RenderContext(80, 24));

$this->assertSame('Package        Downloads', $lines[0]);
$this->assertCount(17, $lines);
Enter fullscreen mode Exit fullscreen mode

Both assertions pass. They also encode my own assumption — that a table configured for fifteen visible rows returns seventeen lines — as if it were the contract. The renderer's actual contract says something different: the returned lines must fit the dimensions the context hands you. I had tested the code against my belief about it, thoroughly, from every angle, at 100% coverage.

Coverage counts the lines you executed. It says nothing about the assumptions you never questioned.

The fix was small once I saw it. What made me see it was not a better test. It was resizing a window.

Dogfooding is boring and it works

Putting the package into a real application took an evening and surfaced more than the height bug:

The empty-state strings were hardcoded English. In an interface that was otherwise entirely in Russian, filtering to nothing produced No matches in the middle of the screen, and the class was final, so a subclass could not fix it either.

Paging moved by the configured window size, which stopped meaning anything as soon as the height came from the layout instead. In a tall terminal, PageDown jumped ten rows in a window showing forty.

To write a status line that said "sorted by Check-in", the calling code had to rebuild its own map of column keys to headers, because the widget knew its columns and would not hand them over. To show the number of matching rows, it had to catch a filter event and keep the count in a variable by reference.

None of these are exotic. Every one of them is the sort of thing you find in the first hour of use and never find by writing more unit tests, because they are not defects in the code — they are defects in the shape of the API, and your tests are written against that same shape.

Then the component itself started leaking

Once my own bugs were out of the way, two more turned up that were not mine.

The first: on exit, every application built on the component left a blank line behind, and a tall frame lost its top row to scrolling even when it would have fit. Tui::stop() moves the cursor to the end of the rendered content and then writes \r\n. The arithmetic was line_count - cursor_row, but line_count is a count and cursor_row is a zero-based index, so that difference already lands one line below the frame. The \r\n then pushed it one further. A five-line frame in a six-row terminal fits together with the shell prompt, and still lost its first line.

The second: a pseudo-terminal whose window size was never set reports 0 0 for stty size. The component's terminal class accepted that as a real size, the renderer floored the content area at one column, and the width check then rejected that single column against the zero columns available. The application died with an uncaught exception and exit code 255. Wrappers around pty.fork(), some CI harnesses and screenshot scripts all leave the window size unset — I hit it while trying to take a screenshot of my own demo.

That second one had a tidy answer sitting in the same framework: Console\Terminal already treats a zero as "size unknown" and falls back with ?:. The TUI terminal did the same fallback with ??, so a zero passed straight through. One character.

Both were merged into 8.1 and shipped in 8.1.5. Writing them up took longer than writing the patches, and that was the right ratio: a one-line diff with no explanation is a guess, and a one-line diff with a reproducing test and a paragraph about why the arithmetic is off is a bug report someone can act on.

What I would tell myself two weeks ago

Full coverage of your assumptions is not coverage. My 92 tests were not bad tests. They were tests written by the same person, on the same day, with the same mental model as the code. They could not have caught the height bug, because the bug lived in the model.

Use the thing. An evening of real use found more than a week of test writing would have. This is not an argument against tests — the tests caught plenty during refactoring, and I would not touch the widget without them. It is an argument against believing that a green suite means anything about the parts of reality you did not think to model.

Read the diffs of what you depend on. A patch release where "nothing broke" is what explained a bug that had been in my package since day one.

Building on an experimental component is a fair trade. The classes carry an experimental marker, which in Symfony terms means a minor release may change the API without a deprecation cycle. In exchange the field is open and the maintainers are paying attention — both of my fixes went in within a day of opening. Pin the minor version for real, ~8.1.0 rather than ^8.1, since those two mean different things. Watch the label in the issue tracker, and read the source of the thing you build on — you will need it anyway, since the documentation page has not been written yet.

Reading that source pays twice. Once for your own code, and once for the bugs you can hand back.


The package is tui-datatable if you want to look at it, though I would not call that the point.

Two weeks ago I was going to end this with a question: is anyone else building on this component? It had zero dependent packages then, and I could not tell whether that was an open field or a warning sign.

The answer arrived while the draft sat in a folder. There are nine dependent packages now. One of them is a bundle with its own DataTableWidget — paging, full-text search through SQLite FTS5, and mouse support the author wired up himself rather than waiting for mouse events to land in core. It has more installs than mine.

So the question is settled, and I like the answer: the ecosystem around this component is assembling faster than the documentation for it.

Top comments (0)