Skip to content

Commit

Permalink
Stricter matches on raised errors in test suite (#1707)
Browse files Browse the repository at this point in the history
Closes #1700.

Partially addresses #1708.
  • Loading branch information
Andrew-S-Rosen authored Feb 15, 2024
1 parent 9c9eb1e commit f5d43ed
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/quacc/calculators/espresso/espresso.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ def _cleanup_params(self) -> None:
"""

if self.kwargs.get("directory"):
raise ValueError("quacc does not support the directory argument.")
raise NotImplementedError("quacc does not support the directory argument.")

self.kwargs["input_data"] = Namelist(self.kwargs.get("input_data"))
self.kwargs["input_data"].to_nested(binary=self._binary, **self.kwargs)
Expand Down
12 changes: 0 additions & 12 deletions src/quacc/recipes/dftb/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@

from ase.calculators.dftb import Dftb

from quacc import SETTINGS
from quacc.runners.ase import run_calc
from quacc.schemas.ase import summarize_run
from quacc.utils.dicts import recursive_dict_merge
from quacc.utils.files import check_logfile

if TYPE_CHECKING:
from pathlib import Path
Expand Down Expand Up @@ -61,14 +59,4 @@ def base_fn(
atoms.calc = Dftb(**calc_flags)
final_atoms = run_calc(atoms, geom_file=GEOM_FILE, copy_files=copy_files)

if SETTINGS.CHECK_CONVERGENCE:
if check_logfile(LOG_FILE, "SCC is NOT converged"):
msg = f"SCC is not converged in {LOG_FILE}"
raise RuntimeError(msg)
if calc_flags.get("Driver_") == "GeometryOptimization" and not check_logfile(
LOG_FILE, "Geometry converged"
):
msg = f"Geometry optimization did not complete in {LOG_FILE}"
raise RuntimeError(msg)

return summarize_run(final_atoms, atoms, additional_fields=additional_fields)
26 changes: 13 additions & 13 deletions tests/core/atoms/test_atoms_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ def test_check_charge_and_spin(os_atoms):
charge, spin_multiplicity = check_charge_and_spin(atoms)
assert charge == 0
assert spin_multiplicity == 2
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(atoms, spin_multiplicity=1)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(
atoms, charge=0, spin_multiplicity=1
)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(atoms, spin_multiplicity=3)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(
atoms, charge=0, spin_multiplicity=3
)
Expand All @@ -113,7 +113,7 @@ def test_check_charge_and_spin(os_atoms):
)
assert charge == 0
assert spin_multiplicity == 4
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(
os_atoms, charge=0, spin_multiplicity=3
)
Expand All @@ -122,19 +122,19 @@ def test_check_charge_and_spin(os_atoms):
charge, spin_multiplicity = check_charge_and_spin(atoms)
assert charge == 0
assert spin_multiplicity == 2
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(atoms, spin_multiplicity=1)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(
atoms, charge=0, spin_multiplicity=1
)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(atoms, spin_multiplicity=3)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(
atoms, charge=0, spin_multiplicity=3
)
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(atoms, charge=-1)

charge, spin_multiplicity = check_charge_and_spin(
Expand All @@ -153,7 +153,7 @@ def test_check_charge_and_spin(os_atoms):
)
assert charge == 0
assert spin_multiplicity == 4
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
charge, spin_multiplicity = check_charge_and_spin(
os_atoms, charge=0, spin_multiplicity=3
)
Expand All @@ -175,7 +175,7 @@ def test_check_charge_and_spin(os_atoms):
atoms = molecule("CH3")
atoms.set_initial_charges([-2, 0, 0, 0])
atoms.set_initial_magnetic_moments([4, 0, 0, 0])
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
check_charge_and_spin(atoms)

atoms = Atoms.fromdict(
Expand All @@ -197,7 +197,7 @@ def test_check_charge_and_spin(os_atoms):
)
assert charge == 0
assert spin_multiplicity == 3
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="not possible for this molecule"):
check_charge_and_spin(atoms, charge=0, spin_multiplicity=2)

atoms = molecule("O2")
Expand Down
12 changes: 9 additions & 3 deletions tests/core/atoms/test_slabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,19 @@ def test_errors():
atoms.set_tags(None)
atoms.center(vacuum=10, axis=2)

with pytest.raises(ValueError):
with pytest.raises(
ValueError, match="Cannot specify both min_distance and find_ads_sites_kwargs"
):
make_adsorbate_structures(
atoms, h2o, min_distance=1.0, find_ads_sites_kwargs={"distance": 1.0}
)
with pytest.raises(ValueError):
with pytest.raises(
ValueError, match="Cannot specify both modes and find_ads_sites_kwargs"
):
make_adsorbate_structures(
atoms, h2o, modes=["ontop"], find_ads_sites_kwargs={"positions": ["ontop"]}
)
with pytest.raises(ValueError):
with pytest.raises(
ValueError, match="All indices in allowed_surface_indices must be in atoms"
):
make_adsorbate_structures(atoms, h2o, allowed_surface_indices=[100])
8 changes: 4 additions & 4 deletions tests/core/calculators/espresso/test_espresso.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def test_espresso_presets_gamma():


def test_espresso_bad_kpts():
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="Cannot specify both kpts and kspacing"):
Espresso(kpts=(1, 1, 1), kspacing=0.5)


Expand All @@ -151,13 +151,13 @@ def test_output_handler(tmp_path, monkeypatch):

test_path = "../test3/test2/test1"
fake_template = EspressoTemplate()
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="because it is not a subpath"):
new_parameters = fake_template._output_handler(parameters, Path())

test_path = "/test3/test2/test1"
parameters["input_data"]["system"]["outdir"] = test_path
fake_template = EspressoTemplate()
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="because it is not a subpath"):
new_parameters = fake_template._output_handler(parameters, Path())

test_path = Path("test3/test2/test1")
Expand All @@ -174,5 +174,5 @@ def test_output_handler(tmp_path, monkeypatch):
def test_bad_calculator_params():
atoms = Atoms(symbols="LiLaOZr")

with pytest.raises(ValueError):
with pytest.raises(NotImplementedError, match="does not support the directory"):
Espresso(input_atoms=atoms, kpts=(1, 1, 1), directory="bad")
9 changes: 7 additions & 2 deletions tests/core/calculators/qchem/test_qchem.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,15 @@ def test_qchem_write_input_basic(tmp_path, monkeypatch, test_atoms):
assert qcinp.as_dict() == ref_qcinp.as_dict()
assert not Path(FILE_DIR / "53.0").exists()

with pytest.raises(NotImplementedError):
with pytest.raises(
NotImplementedError, match="The directory kwarg is not supported"
):
QChem(test_atoms, directory="notsupported")

with pytest.raises(NotImplementedError):
with pytest.raises(
NotImplementedError,
match="Do not specify `molecule` in `qchem_dict_set_params`",
):
calc = QChem(
test_atoms, job_type="freq", qchem_dict_set_params={"molecule": "test"}
)
Expand Down
6 changes: 4 additions & 2 deletions tests/core/calculators/vasp/test_vasp.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ def test_lasph_aggressive():
def test_vdw():
atoms = bulk("Cu")

with pytest.raises(OSError):
with pytest.raises(OSError, match="VASP_VDW setting was not provided"):
Vasp(atoms, xc="beef-vdw")


Expand Down Expand Up @@ -788,7 +788,9 @@ def test_constraints():

atoms = bulk("Cu") * (2, 1, 1)
atoms.set_constraint(FixBondLength(0, 1))
with pytest.raises(ValueError):
with pytest.raises(
ValueError, match="Atoms object has a constraint that is not compatible"
):
calc = Vasp(atoms)


Expand Down
4 changes: 2 additions & 2 deletions tests/core/calculators/vasp/test_vasp_custodian.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def test_run_vasp_custodian(monkeypatch):

run_custodian(vasp_custodian_wall_time=1)

with pytest.raises(ValueError):
with pytest.raises(ValueError, match="Unknown VASP error handler"):
run_custodian(vasp_custodian_handlers="cow")

with pytest.raises(ValueError):
with pytest.raises(ValueError, match="Unknown VASP validator"):
run_custodian(vasp_custodian_validators=["cow"])
8 changes: 4 additions & 4 deletions tests/core/recipes/dftb_recipes/test_dftb_recipes.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_static_job_cu_kpts(tmp_path, monkeypatch):
def test_static_errors(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)

with pytest.raises(RuntimeError):
with pytest.raises(RuntimeError, match="failed with command"):
atoms = molecule("H2O")
static_job(atoms, Hamiltonian_MaxSccIterations=1)

Expand Down Expand Up @@ -152,18 +152,18 @@ def test_relax_job_cu_supercell_cell_relax(tmp_path, monkeypatch):

def test_relax_job_cu_supercell_errors(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)
with pytest.raises(RuntimeError):
with pytest.raises(RuntimeError, match="failed with command"):
atoms = bulk("Cu") * (2, 1, 1)
atoms[0].position += 0.5
relax_job(atoms, kpts=(3, 3, 3), MaxSteps=1, Hamiltonian_MaxSccIterations=100)


def test_child_errors(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)
with pytest.raises(RuntimeError):
with pytest.raises(RuntimeError, match="failed with command"):
atoms = bulk("Cu")
static_job(atoms)

with pytest.raises(RuntimeError):
with pytest.raises(RuntimeError, match="failed with command"):
atoms = bulk("Cu")
relax_job(atoms)
2 changes: 1 addition & 1 deletion tests/core/recipes/espresso_recipes/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def test_static_job_dir_fail(tmp_path, monkeypatch):

atoms = bulk("Si")

with pytest.raises(ValueError):
with pytest.raises(NotImplementedError):
static_job(atoms, directory=Path("fake_path"))


Expand Down
2 changes: 1 addition & 1 deletion tests/core/schemas/test_ase.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_summarize_opt_run(tmp_path, monkeypatch):
dyn.run(steps=5)
traj = read("test.traj", index=":")

with pytest.raises(RuntimeError):
with pytest.raises(RuntimeError, match="Optimization did not converge"):
summarize_opt_run(dyn)

# Make sure info tags are handled appropriately
Expand Down

0 comments on commit f5d43ed

Please sign in to comment.