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: Add tests to the module. #8

Merged
merged 12 commits into from
Mar 18, 2019

Conversation

jhlegarreta
Copy link
Member

Add tests to the module.

@jhlegarreta
Copy link
Member Author

The CI builds will not pass, since the tests are not finished.

@vikramc1 Would you complete the tests?

This will need to be rebased on master after #3 gets merged.

@thewtex
Copy link
Member

thewtex commented Mar 17, 2019

👍

@thewtex
Copy link
Member

thewtex commented Mar 17, 2019

Awesome @jhlegarreta !

Could this please be rebased on master?

Even without full coverage, we can create an issue to improve the tests, and merge this to get the builds going again.

@jhlegarreta
Copy link
Member Author

359e693 rebased on master.

@jhlegarreta jhlegarreta changed the title WIP: ENH: Add tests to the module. ENH: Add tests to the module. Mar 17, 2019
jhlegarreta and others added 7 commits March 18, 2019 15:45
Add tests to the module.
…mage

To address:

  In file included from /home/matt/src/ITKNDReg/test/itkWrapExtrapolateImageFunctionTest.cxx:19:
  /home/matt/src/ITKNDReg/include/itkWrapExtrapolateImageFunction.h:112:8: warning: 'SetInputImage' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
    void SetInputImage(const InputImageType*ptr)
	 ^
  /home/matt/src/ITKNDReg/test/itkWrapExtrapolateImageFunctionTest.cxx:41:3: note: in instantiation of template class 'itk::WrapExtrapolateImageFunction<itk::Image<float, 2>, float>' requested here
    WrapExtrapolateImageFunctionType::Pointer wrapExtrapolateImageFunction =
    ^
  /home/matt/src/ITK/Modules/Core/ImageFunction/include/itkImageFunction.h:111:16: note: overridden virtual function is here
    virtual void SetInputImage(const InputImageType *ptr);
We do not create any libraries (there is no longer an "src" directory).
…ilterTest

This has issues and does not appear to be exercised in the ndreg
repository.
Detected by:

  include/itkMetamorphosisImageRegistrationMethodv4.h:74:8: warning: 'ThreadedGenerateData' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]

ThreadedGenerateData has been replace by DynamicThreadedGenerateData in
ITK 5:

  https://github.com/InsightSoftwareConsortium/ITK/blob/master/Documentation/ITK5MigrationGuide.md
To address:

  include/itkMetamorphosisImageRegistrationMethodv4.h:228:8: warning: 'StartOptimization' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
To address:

  include/itkMetamorphosisImageRegistrationMethodv4.hxx:59:90: warning: instantiation of variable 'itk::NumericTraits<itk::CovariantVector<double, 2> >::One' required here, but no definition is available [-Wundefined-var-template]
To address:

  d:\a\1\s\include\itkMetamorphosisImageRegistrationMethodv4.hxx(144): warning C4101: 'B': unreferenced local variable
@jhlegarreta
Copy link
Member Author

I really did not take the time to looks at the issues of this PR, but now that I look at them, probably the CI failures in the rest of the PRs are related to some of these commits, so thanks @thewtex . This is awesome 👍

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Mar 18, 2019

The module builds fine now ✔️ , but Python packaging seems not to work properly:

error: package directory '_skbuild/linux-x86_64-2.7/cmake-install/itk' does not exist 

May be we are having issues with the pipeline configuration?

@thewtex
Copy link
Member

thewtex commented Mar 18, 2019

Thanks @jhlegarreta -- even these minimal test caught a few bugs!

@vikramc1 more testing would certainly be helpful ;-).

@jhlegarreta yes, can you please rebase the other branches?

@thewtex thewtex merged commit a827c83 into InsightSoftwareConsortium:master Mar 18, 2019
@jhlegarreta
Copy link
Member Author

Sure I can. Thanks @thewtex .

@jhlegarreta jhlegarreta deleted the AddTests branch March 18, 2019 22:52
@jhlegarreta
Copy link
Member Author

jhlegarreta commented Mar 18, 2019

@vikramc1 more testing would certainly be helpful ;-).

@vikramc1 We can definitely help you through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants