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

tests: Refactor testing framework #300

Merged
merged 9 commits into from Jun 19, 2023
Merged

Conversation

LecrisUT
Copy link
Collaborator

@LecrisUT LecrisUT commented Jun 17, 2023

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

Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
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

codecov-commenter commented Jun 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (d6fcbe8) 86.09% compared to head (4aa0806) 86.07%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

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.

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Copy link
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?

test/functional/c/CMakeLists.txt Outdated Show resolved Hide resolved
@LecrisUT
Copy link
Collaborator Author

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.

Signed-off-by: Cristian Le <git@lecris.dev>
@lan496
Copy link
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
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.

Copy link
Collaborator

@atztogo atztogo left a comment

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
Collaborator Author

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
11 checks passed
@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
Labels
enhancement Significant ehancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants