missing test for LAMMPS DumpReader for ValueError when missing coordinates

[orbeckst]

The case when no coordinates following the specified convention in the Timestep are found and a ValueError is raised, is not tested:

mdanalysis/package/MDAnalysis/coordinates/LAMMPS.py

Lines 833 to 838 in 19e675b

	elif not self.lammps_coordinate_convention in convention_to_col_ix:
	raise ValueError(
	f"No coordinates following convention "
	"{self.lammps_coordinate_convention} found in "
	"timestep"
	)

(The incorrect f-string use is fixed in #5053 but a test is still missing.)

[talagayev]

I quickly looked into that and wanted to ask if the test_open_absent_convention_fails test does not already test that ValueError or if it should be modified / a new one should be added?

mdanalysis/testsuite/MDAnalysisTests/coordinates/test_lammps.py

Lines 775 to 781 in 03eef45

	def test_open_absent_convention_fails(convention):
	with pytest.raises(ValueError, match="No coordinates following"):
	mda.Universe(
	LAMMPSDUMP,
	format="LAMMPSDUMP",
	lammps_coordinate_convention=convention,
	)

[spyke7]

Hi @orbeckst @talagayev
I looked into this. And I think that lines 833-838 are already covered by the existing test test_open_absent_convention_fails.
added a debug print statement (right before the raise ValueError):

elif not self.lammps_coordinate_convention in convention_to_col_ix:
    print("DEBUG LINE REACHED")  # Added for testing
    raise ValueError(
        f"No coordinates following convention "
        f"{self.lammps_coordinate_convention} found in "
        "timestep"
    )

the ouput of pytest for this particularly this file was -

testsuite\MDAnalysisTests\coordinates\test_lammps.py::test_open_absent_convention_fails[unscaled] DEBUG LINE REACHED
PASSED
testsuite\MDAnalysisTests\coordinates\test_lammps.py::test_open_absent_convention_fails[unwrapped] DEBUG LINE REACHED
PASSED
testsuite\MDAnalysisTests\coordinates\test_lammps.py::test_open_absent_convention_fails[scaled_unwrapped] DEBUG LINE REACHED
PASSED

[rishitha2827]

Hi, I’d like to work on adding the missing test for this issue. Please let me know if that’s okay.