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: Use macros in itk::SLICImageFilter test #4187

Conversation

jhlegarreta
Copy link
Member

@jhlegarreta jhlegarreta commented Sep 2, 2023

  • STYLE: Prefer using macros for itk::SLICImageFilter basic methods
  • STYLE: Use standard exit return code in itk::SLICImagetFilter test
  • STYLE: Conform to ITK SWG style guidelines in test ending message

PR Checklist

@github-actions github-actions bot added type:Enhancement Improvement of existing methods or implementation type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Segmentation Issues affecting the Segmentation module type:Coverage Code coverage impacts labels Sep 2, 2023
Prefer using macros to exercise `itk::SLICImageFilter` basic methods:
- Exercise basic object methods using the
  `ITK_EXERCISE_BASIC_OBJECT_METHODS`. Remove redundant calls to
  test the filter class and superclass names and to print the filter:
  rely on the basic method exercising macro call.
- Refactor the helper method to return a value as the macro may return a
  value, and rename it to honor its purpose.
- Rename the google test method to honor its contents.
Use standard exit return code in `itk::SLICImagetFilter` test.
Conform to ITK SWG style guidelines in test ending message in
`itk::SLICImageFilter` test: add a message to mark the test end to match
the recommended test ending message.
@jhlegarreta jhlegarreta force-pushed the IncreaseSLICImageFilterCoverage branch from 7ab0f35 to d9f0d5e Compare September 2, 2023 22:15
@jhlegarreta jhlegarreta changed the title ENH: Increase itk::SLICImageFilter coverage STYLE: Use macros in itk::SLICImageFilter test Sep 2, 2023
@@ -140,5 +148,6 @@ itkSLICImageFilterTest(int argc, char * argv[])
}


return 0;
std::cout << "Test finished." << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this print statement above is useful

Copy link
Member Author

@jhlegarreta jhlegarreta Sep 5, 2023

Choose a reason for hiding this comment

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

This was brought in PR #3724 and maybe several other PRs as well; memory was failing me but Dženan made it clear what I believe is one of the benefits of this. Eventually, using a macro that involves the message and the return code (cross-referencing issue #1273) could be beneficial, and bring the debate down to a single line across the whole codebase.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@blowekamp blowekamp left a comment

Choose a reason for hiding this comment

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

Thanks for the clean up!

@dzenanz dzenanz merged commit 586edb5 into InsightSoftwareConsortium:master Sep 6, 2023
5 checks passed
@jhlegarreta jhlegarreta deleted the IncreaseSLICImageFilterCoverage branch September 6, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Segmentation Issues affecting the Segmentation module type:Coverage Code coverage impacts type:Enhancement Improvement of existing methods or implementation 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