Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STYLE: Improve miscellaneous tests' style #4179

Merged

Conversation

jhlegarreta
Copy link
Member

@jhlegarreta jhlegarreta commented Aug 31, 2023

  • STYLE: Declare variables close to where they are used in tests
  • STYLE: Declare and use test image dimensionality as constexpr
  • STYLE: Do not declare a test status variable that is only used once
  • ENH: Allow itkCellInterfaceTest to run as long as possible
  • STYLE: Use exception checking macros in itkVectorExpandImageFilterTest
  • STYLE: Prefer setting the input once all ivars have been set in test

PR Checklist

@github-actions github-actions bot added type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) labels Aug 31, 2023


expander->ResetPipeline();
expander->SetInterpolator(interpolator);
Copy link
Member Author

@jhlegarreta jhlegarreta Aug 31, 2023

Choose a reason for hiding this comment

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

It is weird that I had to add these two lines after the call to update the filter. The previous commit 10b5e6a produced a test failure:
https://open.cdash.org/viewTest.php?onlyfailed&buildid=8953052

It is unclear to me the reason of such failure, despite the test completing its instructions. Even more so this being a seemingly harmless style change. Any thoughts?

Declare variables close to where they are used in tests and initialize
them at declaration.
Declare and use image dimensionality as a constant expression in
miscellaneous tests instead of hard-coding the value across different
statements.
Do not declare a test status variable that is only used once: prefer
using directly the appropriate return value.
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch 2 times, most recently from 751bca2 to e14a731 Compare August 31, 2023 15:21
@github-actions github-actions bot added area:Registration Issues affecting the Registration module area:Video Issues affecting the Video module labels Sep 2, 2023
@jhlegarreta jhlegarreta force-pushed the ImproveMiscTestsStyle branch 5 times, most recently from d2c797a to c179ce3 Compare September 2, 2023 17:29
@github-actions github-actions bot removed area:Registration Issues affecting the Registration module area:Video Issues affecting the Video module labels Sep 2, 2023
Allow `itkCellInterfaceTest` to run as long as possible: store the check
status value in a variable and return such value at the end of the test.
Conform to the ITK SWG guidelines by doing so.
Use exception checking macros in `itkVectorExpandImageFilterTest.cxx`:
- Use the `ITK_TRY_EXPECT_EXCEPTION` macro to check expected exceptions
  when updating filters in lieu of `try/catch` blocks for the sake of
  readability and compactness, and to save typing/avoid boilerplate
  code.
- Only place the code that might raise exceptions within the macro.
- Change the test status variable type to accomodate for the changes
  above.
Prefer setting the input once all ivars have been set in
`itkPatchBasedDenoisingImageFilterDefaultTest.cxx`.
@dzenanz dzenanz merged commit bb3d6d1 into InsightSoftwareConsortium:master Sep 4, 2023
5 checks passed
@jhlegarreta jhlegarreta deleted the ImproveMiscTestsStyle branch September 4, 2023 13:56
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:Segmentation Issues affecting the Segmentation 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.

3 participants