Skip to content

STYLE: Use the itkNameOfTestExecutableMacro macro in tests#2939

Merged
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:UseITKNameOfTestExecutableMacro
Dec 12, 2021
Merged

STYLE: Use the itkNameOfTestExecutableMacro macro in tests#2939
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:UseITKNameOfTestExecutableMacro

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

@jhlegarreta jhlegarreta commented Dec 11, 2021 •
edited
Loading

  • STYLE: Use the itkNameOfTestExecutableMacro macro in tests
  • STYLE: Use argc and argv to name test argument variables

PR Checklist

@jhlegarreta jhlegarreta mentioned this pull request Dec 11, 2021
Merged
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Dec 11, 2021 •
edited
Loading

A few notes:

  • Would probably be best to rename all av (and ac) to argv and argc for the sake of consistency. Done in e17f25e.
  • I noticed that in these files failure return codes are many times set to -1. Done in STYLE: Use exit return codes consistently in tests #2940.
  • Did not improve neither the above nor the style of the missing parameter message.

These may be best addressed in separate PRs as time permits.

@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: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 labels Dec 11, 2021
@jhlegarreta jhlegarreta force-pushed the UseITKNameOfTestExecutableMacro branch 4 times, most recently from d10c480 to 66724a1 Compare December 11, 2021 13:34
@jhlegarreta jhlegarreta mentioned this pull request Dec 11, 2021
Merged
2 tasks
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Dec 11, 2021 •
edited
Loading

/azp run ITK.macOS.Python

@jhlegarreta jhlegarreta force-pushed the UseITKNameOfTestExecutableMacro branch 2 times, most recently from 9be9e2e to d7b81ce Compare December 12, 2021 00:19
@jhlegarreta
STYLE: Use the itkNameOfTestExecutableMacro macro in tests
0abc534 
Use the `itkNameOfTestExecutableMacro` macro to get the test name and
prevent the test from crashing when `argv[0]` is null.

Use the macro for tests that had the name of the test hard-coded.

Add the input argument check for tests that were missing it.
@jhlegarreta jhlegarreta force-pushed the UseITKNameOfTestExecutableMacro branch from d7b81ce to 0abc534 Compare December 12, 2021 00:26
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Dec 12, 2021 •
edited
Loading

As in #2940 itkPyBufferMemoryLeakTest in ITK.macOS.Python is failing. Looks like a recently introduced change is causing it.

@N-Dekker N-Dekker self-requested a review December 12, 2021 11:33
N-Dekker
N-Dekker approved these changes Dec 12, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker left a comment

Choose a reason for hiding this comment

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

👍

@jhlegarreta jhlegarreta force-pushed the UseITKNameOfTestExecutableMacro branch from e17f25e to b0b1039 Compare December 12, 2021 16:09
@jhlegarreta
STYLE: Use argc and argv to name test argument variables
c8096fb 
Use `argc` and `argv` to name test argument variables consistently, and
to stick to the ITK SW Guide coding style guidelines.
@jhlegarreta jhlegarreta force-pushed the UseITKNameOfTestExecutableMacro branch from b0b1039 to c8096fb Compare December 12, 2021 17:11
hjmjohnson
hjmjohnson approved these changes Dec 12, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

LGTM.

@hjmjohnson
Copy link
Copy Markdown
Member

hjmjohnson commented Dec 12, 2021

Failing test is unrelated to this. itkPyBufferMemoryLeakTest

@hjmjohnson hjmjohnson merged commit 03a3af2 into InsightSoftwareConsortium:master Dec 12, 2021
@jhlegarreta jhlegarreta deleted the UseITKNameOfTestExecutableMacro branch December 12, 2021 22:43
@jhlegarreta jhlegarreta mentioned this pull request Dec 22, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hjmjohnson hjmjohnson hjmjohnson approved these changes

@N-Dekker N-Dekker N-Dekker approved these changes

Assignees

No one assigned

Labels

area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jhlegarreta @hjmjohnson @N-Dekker