InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.42k stars 663 forks source link

Add new checking macros #1273

Open jhlegarreta opened 5 years ago

jhlegarreta commented 5 years ago

Description

The ITK SW Guide states in the Messages in Tests subsection of its Coding Style Guide appendix:

Their adoption seems low. May be proposing macros to ease the process, reduce verbosity and reduce boilerplate code would help.

This does also apply to Examples.

Expected behavior

Missing parameter and regression check messages in tests and examples be consistent, e.g.:

if( argc != 3 )
  {
  std::cerr << "Missing parameters." << std::endl;
  std::cerr << "Usage: " << itkNameOfTestExecutableMacro(argv);
  std::cerr << " inputImage outputImage" << std::endl;
  return EXIT_FAILURE;
  }

or

bool tf = colors->SetColor( 0, 0, 0, 0, name );
if( tf != true )
  {
  std::cerr << "Test failed!" << std::endl;
  std::cerr << "Error in itk::ColorTable::SetColor" << std::endl;
  std::cerr << "Expected: " << true << ", but got: "
    << tf << std::endl;
  return EXIT_FAILURE;
  }

Printing the provided argument count and the argument values could also be considered, like in https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/IO/NIFTI/test/itkNiftiReadWriteDirectionTest.cxx#L35

Actual behavior

Missing parameter and regression check messages in tests and examples are not consistent, e.g

  if (argc < 1)
  {
    std::cerr << "Missing Arguments: " << itkNameOfTestExecutableMacro(argv) << std::endl;
    return EXIT_FAILURE;
  }

Cases (Usage vs usage), etc.

and regressions are still less consistent.

Additional Information

Some of the names of the macros, e.g. itkNameOfTestExecutableMacro would need to be changed so that they honor also their use in ITK Examples.

And this would also mean placing these macros in a file other than itkTestingMacros.h, since they would also serve for the Examples.

dzenanz commented 5 years ago

The most logical place would be itkMacro.h. And the bulk of the work would be changing the code to use the new macro.

Also, use of a macro instead of try/catch block in the examples would be confusing for new users, who are familiar with C++ try/catch but not (yet) familiar with ITK conventions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

jhlegarreta commented 3 years ago

Related to the issue is marking the end of tests: https://github.com/InsightSoftwareConsortium/ITK/pull/2196#discussion_r546761209.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

jhlegarreta commented 2 years ago

This is still relevant stale bot.

jhlegarreta commented 1 year ago

In a broader scope, the whole argument checking method could be put into a macro, providing the required and optional argument names as parameters so that they are printed by the macro itself as well.