Skip to content

STYLE: Increase tests style consistency#2652

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
jhlegarreta:ImproveMiscTestsStyle
Jul 20, 2021
Merged

STYLE: Increase tests style consistency#2652
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
jhlegarreta:ImproveMiscTestsStyle

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

@jhlegarreta jhlegarreta commented Jul 18, 2021

Increase tests style consistency:

  • Use the itkNameOfTestExecutableMacro to get the test name and prevent
    the the test from crashing when argv[0] is null.
  • Use the ITK testing macros when calling filter update methods to reduce
    boilerplate code.
  • Increase consistency in the missing argument check message.
  • Use EXIT_SUCCESS and EXIT_FAILURE to return from tests on
    success/failure.
  • Use the ITK_EXERCISE_BASIC_OBJECT_METHODS macro instead of explicitly
    calling the Print method.

Take advantage of the commit to:

  • Remove the argument check when no input parameters are required.
  • Define the required types within the test body.
  • Define the required types once the input argument check has passed.
  • Remove duplicate calls to filter updates.
  • Remove unused expected exception cases.
  • Only enclose within the ITK_TRY_EXPECT_NO_EXCEPTION macro the filter
    update calls that may throw exceptions.
  • Remove some unnecessary printed messages.
  • Remove unnecessary comment lines devoid of relevant content.

PR Checklist

@github-actions github-actions Bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Jul 18, 2021
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch from d2b5e4f to 2694c46 Compare July 18, 2021 18:31
@jhlegarreta jhlegarreta removed the area:Python wrapping Python bindings for a class label Jul 18, 2021
@github-actions github-actions Bot added the area:Python wrapping Python bindings for a class label Jul 18, 2021
@jhlegarreta jhlegarreta marked this pull request as ready for review July 18, 2021 18:33
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Jul 18, 2021

Cross-referencing PR #564.

A few notes:

  • Using the itk::ImageWrite method is out of the scope of this PR.
  • Increasing the consistency for the argc < vs. argc != check can be done in a separate PR (its scope is broader than this PR and requires agreement on the ITK SW Guide).
  • Increasing the consistency in the constant declaration (e.g. pixel dimensionality) is let for a separate PR (its scope is broader than this PR).
  • I did not add the test ending printed messages. That can be done in a separate PR (its scope is broader than this PR and requires agreement on the ITK SW Guide).
  • I did not check all Update calls. That can be done in a separate PR (its scope is broader than this PR).
  • I did not check for the input argument name style in the parameter number check. That can be done in a separate PR (its scope is broader than this PR and requires agreement on the ITK SW Guide).
  • I did not check for comment message styles (except for a few exceptions). That can be done in a separate PR.
  • Modules/IO/JPEG2000/test/itkJPEG2000ImageIORegionOfInterest.cxx and Modules/IO/NIFTI/test/itkExtractSlice.cxx should be appended with the Test label IMHO. Can be done/better in a separate PR.

@jhlegarreta jhlegarreta changed the title STYLE: Make test style more consistent STYLE: Increase tests style consistency Jul 18, 2021
Copy link
Copy Markdown
Contributor

@tbirdso tbirdso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test macro standardization looks great! One typo to fix for CI.

Comment thread Modules/IO/BMP/test/itkBMPImageIOTestPalette.cxx Outdated
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch 2 times, most recently from 76ef46b to 42d6a39 Compare July 18, 2021 21:15
@jhlegarreta jhlegarreta requested a review from tbirdso July 18, 2021 21:22
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch 3 times, most recently from da24c15 to 6524b39 Compare July 18, 2021 23:12
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch 3 times, most recently from 12b3eba to 8d038b6 Compare July 19, 2021 00:43
Increase tests style consistency:
- Use the `itkNameOfTestExecutableMacro` to get the test name and prevent
  the the test from crashing when `argv[0]` is null.
- Use the ITK testing macros when calling filter update methods to reduce
  boilerplate code.
- Increase consistency in the missing argument check message.
- Use `EXIT_SUCCESS` and `EXIT_FAILURE` to return from tests on
  success/failure.
- Use the `ITK_EXERCISE_BASIC_OBJECT_METHODS` macro instead of explicitly
  calling the `Print` method.

Take advantage of the commit to:
- Remove the argument check when no input parameters are required.
- Define the required types within the test body.
- Define the required types once the input argument check has passed.
- Remove duplicate calls to filter updates.
- Remove unused expected exception cases.
- Only enclose within the `ITK_TRY_EXPECT_NO_EXCEPTION` macro the filter
  update calls that may throw exceptions.
- Remove some unnecessary printed messages.
- Remove unnecessary comment lines devoid of relevant content.
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch from 8d038b6 to 11e1f67 Compare July 19, 2021 01:54
Comment thread Modules/Core/Common/test/itkImageRandomIteratorTest2.cxx
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@dzenanz dzenanz merged commit 16cadc0 into InsightSoftwareConsortium:master Jul 20, 2021
@jhlegarreta jhlegarreta deleted the ImproveMiscTestsStyle branch July 20, 2021 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants