ENH: Increase Review module classes' code coverage

[jhlegarreta]

Increase Review module classes' code coverage:

• Exercise the basic object methods using the
  ITK_EXERCISE_BASIC_OBJECT_METHODS macro.
• Test the Set/Get methods using the ITK_TEST_SET_GET_VALUE macro.
• Test the boolean ivars using the ITK_TEST_SET_GET_BOOLEAN macro.

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)

[jhlegarreta]

To be rebased on master once PR #2652 gets merged.

[jhlegarreta]

The coverage increase is not meant to be exhaustive. Further increases can be made in a separate PR.

[jhlegarreta]

/azp run ITK.macOS

[jhlegarreta]

A note on the CI checks: they do not test the Review module. I have not tested the PR locally. So let's hope one of the bots submitting to CDash does test the Review module.

[dzenanz]

Please rebase

[thewtex]

💚 🙏

[dzenanz]

I rebased and pushed to your branch. Also, I am doing a local build with legacy removed and review module on.

[dzenanz]

The force-push fixes unused variable warning, and wrong class name test failure. Both were uncovered by local testing.

[dzenanz]

If the CI comes back green, this is good to merge. I will do so, unless someone raises an objection or beats me to it 😄

[dzenanz]

The last force-push just rebases on current master.

[jhlegarreta]

💯 Thanks Dženan.

[jhlegarreta]

Looks like this introduced a regression failure in itkLabelGeometryImageFilterTest:
https://open.cdash.org/viewBuildError.php?onlydeltap&buildid=7354628

In file included from /home/kitware/DashboardsITK/Modules/Nonunit/Review/test/itkLabelGeometryImageFilterTest.cxx:26:0:
/home/kitware/Dashboards/src/ITK/Modules/Nonunit/Review/test/itkLabelGeometryImageFilterTest.cxx: In function 'int LabelGeometryImageFilterTest(std::__cxx11::string, std::__cxx11::string, std::__cxx11::string, std::__cxx11::string, std::__cxx11::string)':
/home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:69:51: error: 'Superclass' has not been declared
   std::cout << "Name of Superclass = " << object->Superclass::GetNameOfClass() << std::endl;                           \
                                                   ^
[CTest: warning matched] /home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:69:51: note: in definition of macro 'ITK_EXERCISE_BASIC_OBJECT_METHODS'
   std::cout << "Name of Superclass = " << object->Superclass::GetNameOfClass() << std::endl;                           \
                                                   ^~~~~~~~~~
/home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:79:28: error: 'Superclass' has not been declared
   if (!std::strcmp(object->Superclass::GetNameOfClass(), #SuperclassName))                                             \
                            ^
[CTest: warning matched] /home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:79:28: note: in definition of macro 'ITK_EXERCISE_BASIC_OBJECT_METHODS'
   if (!std::strcmp(object->Superclass::GetNameOfClass(), #SuperclassName))                                             \
                            ^~~~~~~~~~

I tried reproducing the issue locally but the test is passing on my Windows 10+VS 2019. The CDash Windows ryzenator site did not complain about it either; only Linux and Mac sites complain about it.

I was hesitant to submit a patch set adding an #include "itkImageToImageFilter.h" as a fix, since it should be already included when calling #include "itkLabelGeometryImageFilter.h", so I'm wondering whether anyone has a better guess.

[dzenanz]

We already encountered this before: #2319 I think I was even able to reproduce it locally when I had Ubuntu 18.04. I recently switched to 20.04.

…

On Thu, Jul 22, 2021 at 10:58 PM Jon Haitz Legarreta Gorroño < ***@***.***> wrote: Looks like this introduced a regression failure in itkLabelGeometryImageFilterTest: https://open.cdash.org/viewBuildError.php?onlydeltap&buildid=7354628 In file included from /home/kitware/DashboardsITK/Modules/Nonunit/Review/test/itkLabelGeometryImageFilterTest.cxx:26:0: /home/kitware/Dashboards/src/ITK/Modules/Nonunit/Review/test/itkLabelGeometryImageFilterTest.cxx: In function 'int LabelGeometryImageFilterTest(std::__cxx11::string, std::__cxx11::string, std::__cxx11::string, std::__cxx11::string, std::__cxx11::string)': /home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:69:51: error: 'Superclass' has not been declared std::cout << "Name of Superclass = " << object->Superclass::GetNameOfClass() << std::endl; \ ^ [CTest: warning matched] /home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:69:51: note: in definition of macro 'ITK_EXERCISE_BASIC_OBJECT_METHODS' std::cout << "Name of Superclass = " << object->Superclass::GetNameOfClass() << std::endl; \ ^~~~~~~~~~ /home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:79:28: error: 'Superclass' has not been declared if (!std::strcmp(object->Superclass::GetNameOfClass(), #SuperclassName)) \ ^ [CTest: warning matched] /home/kitware/DashboardsITK/Modules/Core/TestKernel/include/itkTestingMacros.h:79:28: note: in definition of macro 'ITK_EXERCISE_BASIC_OBJECT_METHODS' if (!std::strcmp(object->Superclass::GetNameOfClass(), #SuperclassName)) \ ^~~~~~~~~~ I tried reproducing the issue locally but the test is passing on my Windows 10+VS 2019. The CDash Windows ryzenator site did not complain about it either; only Linux and Mac complain about it. I was hesitant to submit a patch set adding an #include itkImageToImageFilter.h as a fix, since it should be already included when calling #include itkLabelGeometryImageFilter.h, so I'm wondering whether anyone has a better guess. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#2653 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANVQ6NJNPFI4D7ALYLFE2LTZDLDRANCNFSM5ASQRDIA> .

[Leengit]

Because Superclass is an alias rather than the actual name of the base class, that could be causing trouble for some compilers. For example, often we have to use the typename keyword to refer to an alias defined with using within a class so that the compiler knows that the alias is a type rather than a data member or member function. So, instead of

object->Superclass::GetNameOfClass()

I might have written

static_cast<const typename decltype(*object)::Superclass *>(object)->GetNameOfClass()

with the explicit typename (and perhaps without the const if we are guaranteed that object won't be const).

[jhlegarreta]

@Leengit thanks for chiming in and for the explanation 👍.

One thing that still I do not understand is why the failure does not happen for each and every class' test where the macro is called.

I do not have a Linux machine on which I would be at ease developing ITK at the moment, so I cannot check whether your proposal would work. So it would be great if someone could check it and submit a PR.

[Leengit]

I've tried my own suggestion static_cast<const typename decltype(*object)::Superclass *>(object)->GetNameOfClass() on my Ubuntu 20.04 machine with gcc-10.3.0 and it does not work. The issue appears to be that decltype(*object) is not well defined during the declaration of object; for example when an (inline) method is defined within a class declaration rather than as a separate definition, which is quite common for ITK's templated classes.

[jhlegarreta]

@Leengit thanks for giving it a try. Unfortunately, I haven't got an alternative proposal. There is still something elusive in the issue; also still puzzled as to why it is not present in virtually all tests.

[dzenanz]

I created #2676 to track this.