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

ENH: Increase coverage for itk::FancyString #4280

Conversation

jhlegarreta
Copy link
Member

@jhlegarreta jhlegarreta commented Nov 4, 2023

  • STYLE: Rename test to honor the tested class
  • ENH: Increase coverage for itk::FancyString
  • STYLE: Use testing macros in itkFancyStringTest.cxx

PR Checklist

@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots 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:IO Issues affecting the IO module type:Coverage Code coverage impacts labels Nov 4, 2023
@jhlegarreta jhlegarreta force-pushed the IncreaseFancyStringCoverage branch 3 times, most recently from 7c83fef to 42799ac Compare November 6, 2023 01:30
@jhlegarreta
Copy link
Member Author

jhlegarreta commented Nov 6, 2023

Not sure about the segfault:
https://open.cdash.org/viewTest.php?onlyfailed&buildid=9125424

There are no segfaults in the latest builds.

@jhlegarreta jhlegarreta force-pushed the IncreaseFancyStringCoverage branch from 42799ac to 12aaf8f Compare November 6, 2023 19:36
@jhlegarreta jhlegarreta changed the title ENH: Increase coverage for itk::FrancyString ENH: Increase coverage for itk::FancyString Nov 6, 2023
@dzenanz
Copy link
Member

dzenanz commented Nov 8, 2023

Rebase to resolve conflicts.

Rename test to honor the tested class.
Increase coverage for `itk::FancyString`: exercise the equality
operators.
Use testing macros in `itkFancyStringTest.cxx`:
- Use the `ITK_TRY_EXPECT_EXCEPTION` macro in lieu of `try/catch` blocks
  to check for expected exceptions for the sake of readability and
  compactness, and to save typing/avoid boilerplate code.
@jhlegarreta jhlegarreta force-pushed the IncreaseFancyStringCoverage branch from 12aaf8f to df79d64 Compare November 8, 2023 23:19
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

💯

@dzenanz dzenanz merged commit 2b8ce77 into InsightSoftwareConsortium:master Nov 9, 2023
6 checks passed
@jhlegarreta jhlegarreta deleted the IncreaseFancyStringCoverage branch November 9, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:IO Issues affecting the IO module type:Coverage Code coverage impacts type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots 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