Add unit testing (#84)

* Fixed outputs.py module after improved testing

* Moved testing to test_general.py

Added explicitly input for methods in ModelSystem and testing

Added testing for _set_system_branch_depth and TODO for the functions in these testings which should be implemented in the datamodel

* Added explicit input and fix testing for atoms_state.py

* Added coverage in README

* Added testing for model_method.py classes related with TB

* Added explicitly input in functions for numerical_settings.py and its testing

* Fix imports using abspath to package

* Added sh script to generate coverage.txt in the project

* Deleted coverage.txt file and add it to gitignore

* Added Ahmed changes to the pipeline for comment on pr from coverage

Added uv to pip install in pipeline

* Fixed README with more info on the script, uv

Author: JosePizarro3

.github/workflows/actions.yaml

@@ -1,47 +1,55 @@
 name: install-and-test
 on: [push]
+
+# https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs
+# `contents` is for permission to the contents of the repository.
+# `pull-requests` is for permission to pull request
+permissions:
+  contents: write
+  checks: write
+  pull-requests: write
+
 jobs:
   install-and-test:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v3
-    - name: Set up Python 3.9
-      uses: actions/setup-python@v2
-      with:
-        python-version: 3.9
-    - name: Install dependencies
-      run: |
-        pip install --upgrade pip
-        pip install '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple
-        pip install coverage coveralls
-    - name: mypy
-      run: |
-        python -m mypy --ignore-missing-imports --follow-imports=silent --no-strict-optional src/nomad_simulations tests
-    - name: Test with pytest
-      run: |
-        python -m coverage run -m pytest -sv tests
-    - name: Submit to coveralls
-      continue-on-error: true
-      env:
-        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-      run: |
-        coveralls --service=github
+      - uses: actions/checkout@v3
+      - name: Set up Python 3.9
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.9
+      - name: Install dependencies
+        run: |
+          pip install uv
+          uv pip install -e '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple --system
+      - name: mypy
+        run: |
+          python -m mypy --ignore-missing-imports --follow-imports=silent --no-strict-optional src/nomad_simulations tests
+      - name: Build coverage file
+        run: |
+          pytest --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=src tests | tee pytest-coverage.txt
+      - name: Pytest coverage comment
+        uses: MishaKav/pytest-coverage-comment@main
+        with:
+          pytest-coverage-path: pytest-coverage.txt
+          junitxml-path: pytest.xml
   build-and-install:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v3
-    - name: Set up Python 3.9
-      uses: actions/setup-python@v2
-      with:
-        python-version: 3.9
-    - name: Build the package
-      run: |
-        pip install --upgrade pip
-        pip install build
-        python -m build --sdist
-    - name: Install the package
-      run: |
-        pip install dist/*.tar.gz --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple
+      - uses: actions/checkout@v3
+      - name: Set up Python 3.9
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.9
+      - name: Build the package
+        run: |
+          pip install uv
+          uv pip install --upgrade pip --system
+          uv pip install build --system
+          python -m build --sdist
+      - name: Install the package
+        run: |
+          uv pip install dist/*.tar.gz --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple --system
   ruff-linting:
       runs-on: ubuntu-latest
       steps:

.gitignore

@@ -45,6 +45,7 @@ htmlcov/
 .cache
 nosetests.xml
 coverage.xml
+coverage.txt
 *.cover
 *.py,cover
 .hypothesis/

README.md

@@ -21,11 +21,16 @@ python3.9 -m venv .pyenv
 . .pyenv/bin/activate
 ```
 
+We recommend installing `uv` for fast pip installation of the packages:
+```sh
+pip install uv
+```
+
 Install the `nomad-lab` package:
 
 ```sh
 pip install --upgrade pip
-pip install '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple
+uv pip install '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple
 ```
 
 **Note!**
@@ -37,25 +42,29 @@ sure to include NOMAD's internal package registry (via `--index-url` in the abov
 You can run local tests using the `pytest` package:
 
 ```sh
-python -m pytest -sv
+python -m pytest -sv tests
 ```
 
 where the `-s` and `-v` options toggle the output verbosity.
 
-Our CI/CD pipeline produces a more comprehensive test report using `coverage` and `coveralls` packages.
-To emulate this locally, perform:
+Our CI/CD pipeline produces a more comprehensive test report using `coverage` and `coveralls` packages. We suggest you to generate your own coverage reports locally by doing:
 
 ```sh
 pip install coverage coveralls
-python -m coverage run -m pytest -sv
+python -m pytest --cov=src  tests
+```
+
+You can also run the script to generate a local file `coverage.txt` with the same information by doing:
+```sh
+./scripts/generate_coverage_txt.sh
 ```
 
 ## Development
 
 The plugin is still under development. If you would like to contribute, install the package in editable mode (with the added `-e` flag) with the development dependencies:
 
 ```sh
-pip install -e .[dev] --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple
+uv pip install -e .[dev] --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple
 ```
 
 ### Setting up plugin on your local installation

pyproject.toml

@@ -31,9 +31,9 @@ dependencies = [
 [project.optional-dependencies]
 dev = [
     'mypy==1.0.1',
-    'pytest==3.10.0',
-    'pytest-timeout==1.4.2',
-    'pytest-cov==2.7.1',
+    'pytest',
+    'pytest-timeout',
+    'pytest-cov',
     'ruff',
     "structlog==22.3.0",
     "lxml_html_clean>=0.1.0",

scripts/generate_coverage_txt.sh

@@ -0,0 +1,7 @@
+#!/bin/bash
+
+# Run pytest with coverage
+python -m pytest --cov=src | tee coverage.txt
+
+# Append the generation message
+echo -e "\n\n\nGenerated using './scripts/generate_coverage_txt.sh' in the terminal in the root folder of the project" >> coverage.txt

src/nomad_simulations/atoms_state.py

@@ -28,7 +28,7 @@
 from nomad.datamodel.metainfo.basesections import Entity
 from nomad.datamodel.metainfo.annotations import ELNAnnotation
 
-from .utils import RussellSaundersState
+from nomad_simulations.utils import RussellSaundersState
 
 
 class OrbitalsState(Entity):
@@ -280,7 +280,7 @@ def resolve_degeneracy(self) -> Optional[int]:
             for jj in self.j_quantum_number:
                 if self.mj_quantum_number is not None:
                     mjs = RussellSaundersState.generate_MJs(
-                        self.j_quantum_number[0], rising=True
+                        J=self.j_quantum_number[0], rising=True
                     )
                     degeneracy += len(
                         [mj for mj in mjs if mj in self.mj_quantum_number]
@@ -293,15 +293,15 @@ def normalize(self, archive, logger) -> None:
         super().normalize(archive, logger)
 
         # General checks for physical quantum numbers and symbols
-        if not self.validate_quantum_numbers(logger):
+        if not self.validate_quantum_numbers(logger=logger):
             logger.error('The quantum numbers are not physical.')
             return
 
         # Resolving the quantum numbers and symbols if not available
         for quantum_name in ['l', 'ml', 'ms']:
             for quantum_type in ['number', 'symbol']:
                 quantity = self.resolve_number_and_symbol(
-                    quantum_name, quantum_type, logger
+                    quantum_name=quantum_name, quantum_type=quantum_type, logger=logger
                 )
                 if getattr(self, f'{quantum_name}_quantum_{quantum_type}') is None:
                     setattr(self, f'{quantum_name}_quantum_{quantum_type}', quantity)
@@ -383,7 +383,7 @@ def normalize(self, archive, logger) -> None:
                 self.n_excited_electrons = None
                 self.orbital_ref.degeneracy = 1
             if self.orbital_ref.occupation is None:
-                self.orbital_ref.occupation = self.resolve_occupation(logger)
+                self.orbital_ref.occupation = self.resolve_occupation(logger=logger)
 
 
 class HubbardInteractions(ArchiveSection):
@@ -552,11 +552,11 @@ def normalize(self, archive, logger) -> None:
                 self.u_interaction,
                 self.u_interorbital_interaction,
                 self.j_hunds_coupling,
-            ) = self.resolve_u_interactions(logger)
+            ) = self.resolve_u_interactions(logger=logger)
 
         # If u_effective is not available, calculate it
         if self.u_effective is None:
-            self.u_effective = self.resolve_u_effective(logger)
+            self.u_effective = self.resolve_u_effective(logger=logger)
 
         # Check if length of `orbitals_ref` is the same as the length of `umn`:
         if self.u_matrix is not None and self.orbitals_ref is not None:
@@ -652,6 +652,6 @@ def normalize(self, archive, logger) -> None:
 
         # Get chemical_symbol from atomic_number and viceversa
         if self.chemical_symbol is None:
-            self.chemical_symbol = self.resolve_chemical_symbol(logger)
+            self.chemical_symbol = self.resolve_chemical_symbol(logger=logger)
         if self.atomic_number is None:
-            self.atomic_number = self.resolve_atomic_number(logger)
+            self.atomic_number = self.resolve_atomic_number(logger=logger)

src/nomad_simulations/general.py

@@ -18,18 +18,16 @@
 
 import numpy as np
 from typing import List
-from structlog.stdlib import BoundLogger
 
-from nomad.units import ureg
-from nomad.metainfo import SubSection, Quantity, MEnum, Section, Datetime
+from nomad.metainfo import SubSection, Quantity, Section, Datetime
 from nomad.datamodel.metainfo.annotations import ELNAnnotation
 from nomad.datamodel.data import EntryData
 from nomad.datamodel.metainfo.basesections import Entity, Activity
 
-from .model_system import ModelSystem
-from .model_method import ModelMethod
-from .outputs import Outputs
-from .utils import is_not_representative, get_composition
+from nomad_simulations.model_system import ModelSystem
+from nomad_simulations.model_method import ModelMethod
+from nomad_simulations.outputs import Outputs
+from nomad_simulations.utils import is_not_representative, get_composition
 
 
 class Program(Entity):
@@ -178,7 +176,9 @@ def _set_system_branch_depth(
     ):
         for system_child in system_parent.model_system:
             system_child.branch_depth = branch_depth + 1
-            self._set_system_branch_depth(system_child, branch_depth + 1)
+            self._set_system_branch_depth(
+                system_parent=system_child, branch_depth=branch_depth + 1
+            )
 
     def resolve_composition_formula(self, system_parent: ModelSystem) -> None:
         """Determine and set the composition formula for `system_parent` and all of its
@@ -217,7 +217,9 @@ def set_composition_formula(
                     for subsystem in subsystems
                 ]
             if system.composition_formula is None:
-                system.composition_formula = get_composition(subsystem_labels)
+                system.composition_formula = get_composition(
+                    children_names=subsystem_labels
+                )
 
         def get_composition_recurs(system: ModelSystem, atom_labels: List[str]) -> None:
             """Traverse the system hierarchy downward and set the branch composition for
@@ -229,10 +231,12 @@ def get_composition_recurs(system: ModelSystem, atom_labels: List[str]) -> None:
                 to the atom indices stored in system.
             """
             subsystems = system.model_system
-            set_composition_formula(system, subsystems, atom_labels)
+            set_composition_formula(
+                system=system, subsystems=subsystems, atom_labels=atom_labels
+            )
             if subsystems:
                 for subsystem in subsystems:
-                    get_composition_recurs(subsystem, atom_labels)
+                    get_composition_recurs(system=subsystem, atom_labels=atom_labels)
 
         atoms_state = (
             system_parent.cell[0].atoms_state if system_parent.cell is not None else []
@@ -242,7 +246,7 @@ def get_composition_recurs(system: ModelSystem, atom_labels: List[str]) -> None:
             if atoms_state is not None
             else []
         )
-        get_composition_recurs(system_parent, atom_labels)
+        get_composition_recurs(system=system_parent, atom_labels=atom_labels)
 
     def normalize(self, archive, logger) -> None:
         super(EntryData, self).normalize(archive, logger)
@@ -263,8 +267,8 @@ def normalize(self, archive, logger) -> None:
             system_parent.branch_depth = 0
             if len(system_parent.model_system) == 0:
                 continue
-            self._set_system_branch_depth(system_parent)
+            self._set_system_branch_depth(system_parent=system_parent)
 
-            if is_not_representative(system_parent, logger):
+            if is_not_representative(model_system=system_parent, logger=logger):
                 continue
-            self.resolve_composition_formula(system_parent)
+            self.resolve_composition_formula(system_parent=system_parent)