Something like https://testanything.org/ could be used to add some vital unit tests to the code. This task's goal should be to find a suitable unit testing framework, and to add tests for the formatter function (it is self contained and it should be easier to add tests for it).
Description
Description
Details
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | elukey | T147432 Varnishkafka testing framework | |||
| Resolved | elukey | T147440 Evaluate a unit testing framework and add tests for the formatter function |
Event Timeline
Comment Actions
Goal is to apply unit testing to 1 function to start, "format function" seems the easiest to get started with.
https://github.com/wikimedia/varnishkafka/blob/master/varnishkafka.c#L748
- remove format function shared state
- add unit tests
Could be that unit testing requires too much a refactor and we just don't do it. Timeboxing to a week.
Comment Actions
Change 322257 had a related patch set uploaded (by Elukey):
[WIP] Refactor the parsing functions out of the main C file
Comment Actions
Change 322256 had a related patch set uploaded (by Elukey):
Move definitions to header files for a better code readability
Comment Actions
Rationale of the changes: I'd like to separate functionalities in multiple .c/.h modules that can be unit tested separately, and the simplest ones seems to be the parsing and string manipulation functions. The parsing functions do not allocate new memory as far I understood since they work on a pre-allocated "scratch" buffer, so it seems natural to unit test them. The Unit test framework has not been decided yet, these changes are the prerequisite imho before starting to think about it.
I have also tested the changes with https://gerrit.wikimedia.org/r/#/admin/projects/operations/software/varnish/varnishkafka/testing (Docker image for basic vk integration testing) and everything looks good.
Comment Actions
I tried to refactor the code to make varnishkafka's source modular, with separate modules to unit test separately. Some of the work is depicted in:
https://gerrit.wikimedia.org/r/#/c/314662/
https://gerrit.wikimedia.org/r/#/c/322256/
https://gerrit.wikimedia.org/r/#/c/322257/
I also have some unpublished and unfinished work on my laptop, but I decided to stop. The main goal of this task was to establish in a fixed amount of time how feasible and difficult it would have been to add unit tests, and I've spent quite a bit of time trying. I thought I had the perfect solution and I even got to the point in which the makefile was able to run make check (http://check.sourceforge.net/). While adding some unit tests, I found global variables (like conf) used everywhere that I didn't notice before, plus a lot of inter-dependencies between functions and structs making difficult to separate them in different modules/headers. The work is doable but it would require a lot of time, not only development but also code reviews and tests.
I had a chat with my team about the benefits of a more tested varnishkafka vs the amount of engineering time spent on it instead of working on other important goals, and the outcome is that pursuing this road is not worth it. I managed to understand deeply more hidden features of the code so the time spent on this task was good, but overall I don't think that spending more it on it will be wise.
My next efforts will concentrate on empowering the integration testing suite.
Comment Actions
Change 322256 abandoned by Elukey:
Move definitions to header files for a better code readability
Reason:
Summary in https://phabricator.wikimedia.org/T147440#2841716
Comment Actions
Change 322257 abandoned by Elukey:
Refactor the parsing functions out of the main C file
Reason:
Summary in https://phabricator.wikimedia.org/T147440#2841716