Broken unit tests

boud at valdyas.org boud at valdyas.org
Tue Aug 21 15:43:44 BST 2018


Out of all tests marked with krita_add_broken_unit_test, nine are of the 
kind where running manually and eyeballing images is relevant. Three of 
those get their images from some place else, so it's unlikely anyone but 
Dmitry will ever look at them.

The rest of the tests marked thus were real (though sometimes useless) 
unittests showing problems. I've re-added most of those and marked the 
broken results with QEXPECT_FAIL, so we can at least see the problems. 
Some are so broken that they hang or segfault, and that needs looking 
into as well. These tests are real problems that should be fixed so we 
can run the full set again.

Again: only a fraction of the tests that were marked as 
krita_add_broken_unit_test were of the kind where manually running them 
and eyeballing the results could help when refactoring. The others are 
all of the kind that should be run every time to avoid regressions.

*They should not stay hidden.*

Here's the full overview:
-------------------------

Broken Images
~~~~~~~~~~~~~

* Broken Image Comparison -- these tests should be run manually and the 
results eyeballed. We
need some kind of convenience applications for that.

          53 - libs-flake-TestSvgText (Failed)
          54 - libs-flake-TestSvgTextCloned (Failed)
          55 - libs-flake-TestSvgTextRoundTrip (Failed)
         265 - plugins-tools-basictools-MoveStrokeTest (Failed)

* This one also does image comparison, but ASSERTS because a certain 
preset cannot be found.

         216 - libs-ui-FreehandStrokeTest (Child aborted)

* This one does image comparison but aborts using qFatal on the first 
different pixel, making it pretty much
useless. Not aborting causes an endless loop.

         218 - libs-ui-KisPaintOnTransparencyMaskTest (Child aborted)

* These are not unittests: these tests need images from a second repo, 
and then the results need to be
eyeballed manually.

         168 - libs-image-kis_transform_mask_test (Failed)
         177 - libs-image-kis_layer_styles_test (Failed)
         259 - plugins-impex-psd-kis_psd_test (Failed)

For the above tests, we should create a new system to run the test code 
and compare images, like we had for odf documents back in the calligra 
days.


Broken Tests
~~~~~~~~~~~~

These unitests were/are really broken. They are enabled again, and the 
faulty results marked with QEXPECT_FAIL. Maybe then someone will check 
them. These real unitests that need to be run regularly.

         166 - libs-image-kis_paint_device_test (Failed)
         171 - libs-image-kis_queues_progress_updater_test (Failed)
         176 - libs-image-kis_image_animation_interface_test (Failed)
         186 - libs-image-tiles3-kis_tile_data_pooler_test (Failed)
         213 - libs-ui-kis_prescaled_projection_test (Failed) <<<< this 
one also contains broken qimage comparisons, in addition to real tests
         226 - libs-ui-KisCategoriesMapperTest (Failed)
         227 - libs-ui-KisAslLayerStyleSerializerTest (Failed) <<<< this 
serializes a qdomdocument to a string, and then expects that the 
attributes will be in the same order

These unittests were rewritten so they run now:

         185 - libs-image-tiles3-kis_store_limits_test (Failed)
         228 - libs-ui-kis_animation_importer_test (Child aborted) <<<< 
This one triggers a safe assert in KisDocument.
         170 - libs-image-kis_update_scheduler_test (Child aborted) <<<< 
crashed because the test was not updated following an api change.

These unittests run after adding some extra fuzziness, but it's 
debatable whether they shouldn't be part of the set of manual visual 
compare tests

         219 - libs-ui-FillProcessingVisitorTest (Failed)

These tests hang and cannot be enabled until they are fixed:

         211 - libs-ui-kis_node_model_test (Child aborted)
         It hangs in testSubstituteRootNode(); I have disabled this for 
now, but it probably shows a real problem. But now at least the other 
tests run.

         212 - libs-ui-kis_shape_controller_test (Child aborted)
         it hangs both in testAddSelectionMasksNoActivation and in the 
destructor: the KisImageSP is still busy, so everything waits for that. 
This probably shows real problems. And there are QEXPECT_FAIL failures.

         223 - libs-ui-KisDummiesFacadeTest (Child aborted)
         hangs for the same reason as KisShapeControllerTest, since both 
tests use the same base test.

These tests are completely broken: it wants to create a KisMainWindow, 
which a unittest should never need to do. It's a unittest, not an 
integration test. I guess they weren't as broken before they were marked 
with krita_add_broken_unit_test, but detoriated even more because nobody 
ever looked at them.

I also consider them useless, the things they test should be tested at a 
lower level.

         221 - libs-ui-KisSelectionManagerTest (Child aborted)
         222 - libs-ui-KisNodeManagerTest (Child aborted)
         224 - libs-ui-KisZoomAndPanTest (Child aborted)
         225 - libs-ui-KisActionManagerTest (Child aborted)

This one crashes, and I'm too tired to look into it. Probably the 
correct way of handling frames changed, and nobody updated the test 
because, being marked with krita_add_broken_unit_test, nobody noticed...

         229 - libs-ui-kis_animation_frame_cache_test (Child aborted)


On Unittests
------------

Ideally, a unittest tests a single unit: that is a class. It should not 
test complex interaction between classes.

A unittest should certainly never:

* show a gui
* do benchmarking

For that sort of testing, we should write a different kind of test. Like 
the old and unported scratchpad tester, or the kofiledialog tester 
application. Or benchmarks. They should not be part of a "make test" 
run.


Boudewijn


More information about the kimageshop mailing list