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

Improve test coverage #83

Merged
merged 10 commits into from
Jun 9, 2023
Merged

Improve test coverage #83

merged 10 commits into from
Jun 9, 2023

Conversation

patrick-austin
Copy link
Contributor

@patrick-austin patrick-austin commented Apr 12, 2023

Changes

  • Added Codecov to the repo
  • Extended use of assertRaises and fixed broken asserts (checking the wrong strings but doing it inside the raises block, leading to the test passing when it should've failed)
  • Added test of MuAIRSS batch (for UEP method)
  • Added tests for uep plot methods (line and plane), this required a structure with more than two atoms (in order to form a plane) so added Si8 files
  • Added read warning for pm-nq
  • Added symmetry tests
  • Added tests for uep/charged and hfine. This required a structure with spin, so used YbCuAs2. In order to get NMR data correctly had to remove casting datatype of species to S2

Notable uncovered areas

In addition to numerous exceptions and minor branches that are not covered (i.e. raising an exception if a file that should always be present is missing), the following areas have functional code that is currently untested but could be:

  • calculate/uep/charged.py:ChargeDistribution 179-195,218-242
  • calculate/hfine.py:compute_hfine_tensor ~30% functional branches uncovered
  • data/dftb_pars/dftb_pars.py ~17% uncovered
  • io/castep.py various parse_functions are either unused or never called in the current tests
  • io/uep.py from the coverage looks like second half of run() is never reached - also complicated by the fact this is being used to mock the calc attribute for ASE atoms, needs further analysis
  • quantum/vibrational/average.py "montecarlo" branch, dftb_pbc branch, castep branch...
  • schemas.py some validations not tested

DFTB tests don't run, as _RUN_DFTB is hardcoded False. Haven't looked into this too much yet. Don't know if it's more feasible to try and install DFTB+ on the GHA runners or try and increase coverage by unit testing the following (currently uncovered) functions:

  • io/output.py:write_phonon_reports
  • quantum/main.py:asephonons_entry
  • quantum/vibrational/phonons.py:ase_phonons_calc

The following files have no coverage, and are not called anywhere else in the codebase. Could write tests for them, but is it worth it for code that is never used? May have value as library functions but it's not obvious to me if they do.

Following discussion with @leandro-liborio will keep the following:

  • calculate/uep/charged.py:ChargeDistrubution.Hfine
  • dipolar/field.py

And remove the following:

  • io/output.py:write_tensors
  • quantum/grid.py
  • quantum/reports.py
  • utils.py:create_plane_grid

#48

@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@24b25cd). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #83   +/-   ##
=======================================
  Coverage        ?   74.23%           
=======================================
  Files           ?       25           
  Lines           ?     2208           
  Branches        ?      351           
=======================================
  Hits            ?     1639           
  Misses          ?      480           
  Partials        ?       89           

@patrick-austin patrick-austin marked this pull request as ready for review June 9, 2023 11:29
@patrick-austin patrick-austin merged commit e35c8b2 into main Jun 9, 2023
@patrick-austin patrick-austin mentioned this pull request Nov 24, 2023
5 tasks
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.

1 participant