Skip to content

Add fortran tests and clean cmake#226

Merged
atztogo merged 6 commits intospglib:developfrom
LecrisUT:ctest-fortran
Jan 29, 2023
Merged

Add fortran tests and clean cmake#226
atztogo merged 6 commits intospglib:developfrom
LecrisUT:ctest-fortran

Conversation

@LecrisUT
Copy link
Copy Markdown
Collaborator

@LecrisUT LecrisUT commented Jan 28, 2023 •
edited
Loading

@atztogo @lan496

Here is the skeleton for the Fortran tests. Simply print to stdout/stderr and return non-zero for any failed test, and make sure the file name and function are identical.

I have also made a minor fix in the find_package(Python), switching it to Python3 for easier packaging in RPM (waiting on scikit-build/scikit-build#834 to finish that).

You should have write access to my branch, so feel free to just push commits on top of it.

Edit: Previous tests had some issues with sanitizer flags, which I had cleaned up in a separate branch, so I decided to just add these ones in as well. So all together this PR does:

  • Adds a ctest skeleton for fortran tests
  • Removes the separate targets for shared and static libraries. Instead use spglib_SHARED_LIBS to switch between either of those.
    • Reasoning: static libraries would only be useful if spglib is bundled in a project. Otherwise, for local development or production it should be using shared libraries
    • I've added a default value to static libraries if spglib is bundled, otherwise build shared (similar to spglib_WITH_TESTS)
  • Adds namespaced alias targets and namespaced options
    • This is similarly to the one above, if the project is a bundled project, it should not be injected by the options of the parent project
    • This is still in rc, and there has been some previous changes to these flags, when I made this project importable, so it shouldn't affect much
  • Use FindPython3 for better compatibility with RPM packaging

Closes #225, #122

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 28, 2023 •
edited by codecov bot
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.34%. Comparing base (aee574d) to head (33c0929).
⚠️ Report is 723 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop     #226   +/-   ##
========================================
  Coverage    93.34%   93.34%           
========================================
  Files           15       15           
  Lines          902      902           
========================================
  Hits           842      842           
  Misses          60       60

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LecrisUT
Correct cmake FindPython and use Python3
c1b5d65 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add Fortran test skeleton
545e2fc 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Cleanup cmake
057664d 
- Move to single shared/static targets defined by `BUILD_SHARED_LIBS=ON/OFF`
- Cleanup cmake to use modern standards, e.g. using `target_compile_definitions`
- Moved pkg-config template file to cmake folder

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Fix fortran mod install directory
7fc0647 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add namespace alias and namespace options
125f3bf 
This will make it possible for external projects to include this library via `FetchContent` and equivalent.

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add spglib_SHARED_LIBS
33c0929 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT LecrisUT changed the title Add fortran tests Add fortran tests and clean cmake Jan 28, 2023
@LecrisUT
Copy link
Copy Markdown
Collaborator Author

LecrisUT commented Jan 28, 2023 •
edited
Loading

Ok, since the last tests failed, I think I will just add the other fixes I had lying around. I'll edit the top comment for more details.

@LecrisUT LecrisUT mentioned this pull request Jan 28, 2023
Closed
lan496
lan496 approved these changes Jan 29, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@lan496 lan496 left a comment

Choose a reason for hiding this comment

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

@LecrisUT, many thanks! I see some configurations in CMakeLists.txt are moved to src/CMakeLists.txt.

@atztogo I am not a Fortran person. So, can you check and merge this PR?

@atztogo atztogo merged commit 4b72ed9 into spglib:develop Jan 29, 2023
@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Jan 29, 2023

Merged. Thanks @LecrisUT. This is really great.
I just started to learn how to write tests in C (thanks @lan496 for the google test and @LecrisUT for ctest). It was good for me to know that fortran tests can also be handled by ctest.

@LecrisUT LecrisUT deleted the ctest-fortran branch January 29, 2023 12:05
@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Jan 30, 2023

@LecrisUT, do you know the license of c_interface_module.F90? If it is from Fortran wiki or it is unclear, it is better not to use it. The license of full Fortran wiki is GPL, https://fortranwiki.org/fortran/show/Fortran+Wiki, though not sure for a part of it.

@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Jan 30, 2023

@LecrisUT. Sorry, it seems public domain about source code on Fortran wiki, https://fortranwiki.org/fortran/show/Copyrights.

@LecrisUT
Copy link
Copy Markdown
Collaborator Author

LecrisUT commented Jan 30, 2023

Yeah, that's what I've found as well. As far as I understand there are no copyright or license restrictions with those.

@atztogo atztogo mentioned this pull request Feb 3, 2023
Closed
@LecrisUT LecrisUT added this to the 2.1 milestone Feb 21, 2023
@LecrisUT LecrisUT added the enhancement Significant ehancements label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lan496 lan496 lan496 approved these changes

Assignees

No one assigned

Labels

enhancement Significant ehancements

Projects

None yet

Milestone

2.1

Development

Successfully merging this pull request may close these issues.

Fortran interface test using ctest

4 participants

@LecrisUT @codecov-commenter @atztogo @lan496