/Modules/ImageStatistics/Testing/mitkImageStatisticsContainerTest.cpp
139	What is this used for?
196	You could also test for `GetStatisticsForTimeStep` with a non-existing time step.
216	I don't think that this is the best way to do it because you will not catch bugs that do not change the length (e.g. if the printed information says `TimeSteps: 0` instead of `TimeSteps: 5`) or if the time geometry is correct. Maybe @floca can help?
221	You should add some statistics data (like in the test before) and clone after that to see if the `m_TimeStepMap` was correctly cloned (e.g. by using `TimeStepExists` or `GetStatisticsForTimeStep` with the cloned container).
233	Did you mean `m_statisticsObject3`? By the way, the naming convention would be to use a capital letter after `m_`, such as `m_StatisticsObject`.
253	What is your aim here? Maybe you can comment on it, stating something like Setting statistics for the same time step will overwrite existing statistics or Setting the same statistics for different time steps will only add the statistics name once. Maybe check, if the correct statistics name is stored (`Test3`)?
261	What is your aim here? What happens is, that you are adding three containers, but only `m_statisticsContainer` actually contains a statistics object. So only the single statistics object has valid time steps and therefore its name is added to the custom statistics names. I think it would be more interesting to actually to have at least two statistics object with different custom names (and maybe push one container without statistics / custom names). But then please comment what is happening, as mentioned above.

This commit now has outstanding concerns.Apr 16 2020, 12:39 PM

floca added inline comments.Apr 16 2020, 1:10 PM

/Modules/ImageStatistics/Testing/mitkImageStatisticsContainerTest.cpp
216	@kalali Is correct. But it is not trival as the print out is not stable (due to pointer addresses encoded in the print out). If it would be stable you could just store the result ones, check if its ok and then just compeare strings. Now you would have to build something like a regex with wildcards for all irrelevant changes. To be realistic. Testing the print out itself is a very nice to have compared to other areas where one would like to have code coverage. So I would just check for now, that print() doesn't throw/fail if called (e.g. on an empty container or a populated one). And skip the rest for now.