Page MenuHomePhabricator

Improve design and tests on src/renderer.js
Closed, DuplicatePublic

Description

We've covered a big part of it with tests, but there is a lot of hidden logic inside the renderer that may break unnoticed.

The show function, for example, is responsible for too many things. It is too complicated as it accepts seemingly unrelated parameters, creates a layout, layouts a preview, binds some behavior, etc.
Binding a behavior as part of showing a preview hasn't been tested. In general functions make use of other functions in the file without making them explicit dependencies, unlike data passed to functions. Passing dependent functions to other functions is also not practical as the function signature gets even more complicated.
The createThumbnail function has two many code paths. It takes enourmous effort in order to understand the function correctly and change something in it. Although tests try to cover all cases, we are only creating one case for each code path. getClasses is another such function.
What I'd like to see is we break up functions into small chunks that do one thing well and we test those funtions with random input. Testing monstrous functions with random input will be a major pain. I suggest we use a property-based testing library, perhaps JSVerify, for generating random test data.

I suggest creating subtasks for code that is going to be revised, to slowly chip away at this.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 17 2017, 10:17 AM

Duplicate of T165036: Add coverage for test scenarios in src/renderer.js unit tests ?