Skip to content

tests: Refactor testing framework#300

Merged
LecrisUT merged 9 commits intospglib:developfrom
LecrisUT:test-framework
Jun 19, 2023
Merged

tests: Refactor testing framework#300
LecrisUT merged 9 commits intospglib:developfrom
LecrisUT:test-framework

Conversation

@LecrisUT
Copy link
Copy Markdown
Collaborator

@LecrisUT LecrisUT commented Jun 17, 2023 •
edited
Loading

This PR simply moves the tests around to either unit_tests, integration_tests (currently none) or functional_tests.

TODO:

  • Check if there are any other non-functional tests in the list

LecrisUT added 4 commits June 17, 2023 19:16
@LecrisUT
Add basic test framework structure
792db58 
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT
Add basic ctest framework
d3adaa9 
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT
Move c_api functional tests correspondingly
a27d0f9 
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT
Partially rename google test suite
b62fa88 
This is to compy with the guidelines in
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore

Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT LecrisUT added the enhancement Significant ehancements label Jun 17, 2023
@LecrisUT LecrisUT requested a review from lan496 June 17, 2023 20:17
@LecrisUT LecrisUT self-assigned this Jun 17, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 17, 2023 •
edited by codecov bot
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.07%. Comparing base (d6fcbe8) to head (4aa0806).
⚠️ Report is 586 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #300      +/-   ##
===========================================
- Coverage    86.09%   86.07%   -0.02%     
===========================================
  Files           23       23              
  Lines         6069     6069              
===========================================
- Hits          5225     5224       -1     
- Misses         844      845       +1
Flag Coverage Δ
c_api 76.48% <ø> (-0.02%) ⬇️
fortran_api 37.46% <ø> (ø)
python_api 82.83% <ø> (ø)
unit_tests 76.48% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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 added 4 commits June 17, 2023 22:45
@LecrisUT
Move some unit tests to their proper location
d7a6a43 
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT
Add unit tests to codecov
151e3e9 
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT
Move fortran functional tests
238146a 
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT
Move python functional tests
0a5f225 
Signed-off-by: Cristian Le <git@lecris.dev>
lan496
lan496 requested changes Jun 18, 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.

Can you explain the difference between test/functional and test/unit?

Comment thread test/functional/c/CMakeLists.txt Outdated
@LecrisUT
Copy link
Copy Markdown
Collaborator Author

LecrisUT commented Jun 18, 2023

Can you explain the difference between test/functional and test/unit?

I wrote it in another PR, but:

Unit/integration/functional test

unit test

  • runs a single self-contained internal function, e.g. constructing and destructing the lattice object
  • it does not touch any physical aspects
  • used to debug compilation, segmentation issues etc.
  • the main thing to investigate in codecov

functional test

  • runs the full calculation as the user would be doing
  • requires input data
  • tests the physical aspects of the library

integration test (yes, there's one more technically)

  • middle step between unit and functional test. Runs multiple fumctions covered by unit tests or the higher level functions
  • tests if the data is transfered correctly between the functions, mainly for segmentation faults and unexpected crashes in the manipulation of data

So in this case, because most of the tests are calling through spglib.h directly most of them are functional tests (what the user would be using)

  • Example of unit-tests: constructor/destructor, individual maths operations, individual operations like search symmetry (constructors/destructors must be handled by the test fixture, not the api)
  • Example of functional test: calling api functions like spg_get_hall_number_from_symmetry

There are edge-cases like spg_get_spacegroup_type where it is an api, but it only calls constructors, and because there is no complicated logic that calls other units, I consider this as a unit test.

@LecrisUT
Prepend test_ prefix back to the test_suite files
4aa0806 
Signed-off-by: Cristian Le <git@lecris.dev>
@lan496
Copy link
Copy Markdown
Member

lan496 commented Jun 19, 2023

Thanks for your explanation. I confused the distinction between unit and functional.

@lan496 lan496 requested a review from atztogo June 19, 2023 00:48
@lan496
Copy link
Copy Markdown
Member

lan496 commented Jun 19, 2023

@atztogo LGTM. Do you have any comment? This PR touches a lot of files, so I think it is necessary to consult on you.

atztogo
atztogo approved these changes Jun 19, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@atztogo atztogo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @LecrisUT and @lan496. I confirmed pytest works on my laptop, by which I am satisfied due to your agreement on other tests. Reviewing more than that requires me to take time to understand, but no problem for me to let it go than blocking.

@LecrisUT
Copy link
Copy Markdown
Collaborator Author

LecrisUT commented Jun 19, 2023

Yeah, but this PR only moves and renames things so it's not effecting anything atm. #303 is where things start changing

@LecrisUT LecrisUT merged commit 3e8666e into spglib:develop Jun 19, 2023
@LecrisUT LecrisUT deleted the test-framework branch June 19, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atztogo atztogo atztogo approved these changes

@lan496 lan496 lan496 approved these changes

Assignees

@LecrisUT LecrisUT

Labels

enhancement Significant ehancements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LecrisUT @codecov-commenter @lan496 @atztogo