Page MenuHomePhabricator

Review FileWriter test
Closed, WontfixPublic

Description

Several FileWriter tests do not actually test the actual writing, very common is testing for an itk::Exception while trying to write the file to an unwriteable location (like "/usr/bin/").

Also for the NRRD format, which is to be the central image format in MITK, implement an comprehensive NRRDFileWriterTest.

Event Timeline

The mitkImageWriterTest simply tests storing an input image (needed as argv to be passed) in .mhd, .nrrd format and after storing a file it is only tested that the file was written, the check if the stored information is valid is missing.

Proposed design of the new test:

  • Use mitk::ImageGenerator to create a 4D Image
  • Get the list of extensions ( vector of strings available through the ImageWriter ) - make a selection of the extensions for testing 4D, 3D and 2D Image storing ( for exmaple .png, .bmp should be excluded from the tested extensions for the 4D Image )
  • test storing 4D Images ( file was written, test validity by reading it in again and comparing to the generated image )
  • extract a volume and test storing 3D Image
  • extract a slice and test storing 2D Image

The test should have found : T12001 and T12657

but it checks only if a file of the filename of the previously stored image can be read in again and shows [PASSED] if yes.

I stumbled upon the same issue while searching for an example of a writer test. The following tests are all doing the same(actually nothing):

  • mitkImageWriterTest
  • mitkPointSetWriterTest
  • mitkSurfaceVtkWriterTest
  • mitkUnstructuredGridVtkWriter

With the newly implemented Equal methods, the testing of saving/loading images will be easier, however we still should design a proper checking mechanism for testing that the file was correctly stored at the desired location:

How to test?

  • load the image again and compare with Equal ( <- here it could fail because of some error in the reader, i don't know if this is the proper testing way )
  • ensure the file (or a series of files when using the SeriesWriter) saved exists in the target directory <- is it enough to test if the file(s) exist?

@Thomas: is there a proper way for writer testing? You have read a lot about testing concepts.

Just checking if the file exists is not a relevant/useful test, since this ensures almost nothing AND is a troublesome integration test which could easy fail (due to user rights, hardware conditions etc.).

For an ideal UNIT test, you want to isolate the functionality as much as possible. A solution for this would be implementing a virtual hard disk (so called moc object). However, we don't have a testing framework and would have to implement this by ourselves which is also troublesome in C++.

You want 3 things:

  1. Trustworthiness - i.e. the test should only fail, if something at the respective writer code is wrong or some other directly related code like mitkImage. Very important in this case and our main issue.
  1. Readability - always the most important, because if you can't read it, you can't trust it and you can't maintain it.
  1. Maintainability - extensible, clear structure, etc.. This comes indirectly with readability in my opinion. If not, the test is to much effort/to big anyway.

I know that Sascha and Alfred worked on a method to create a temporary directory for testing. In my opinion, we can give it a try to write a simple integration test, as you suggested. Write out an image (preferably very small 3x3, 5x5), read it and compare it using the new compare methods. If we have too much trouble with temporary directories, we can still implement a moc object. If we need a more tolerant comparison (e.g. the image is float and the last digits are lost during reading/writing), we can implement it in the fashion of the Equal methods.

hering added a subscriber: hering.
kislinsk added a project: Auto-closed.
kislinsk added a subscriber: kislinsk.

Hi there! 🙂

This task was auto-closed according to our Task Lifecycle Management.
Please follow this link for more information and don't forget that you are encouraged to reasonable re-open tasks to revive them. 🚑

Best wishes,
The MITK devs