Uh oh!
There was an error while loading. Please reload this page.
Add tests for previously uncovered utility, dataset, and metadatable … - #7961
Add tests for previously uncovered utility, dataset, and metadatable …#7961Jens Hedegaard Nielsen (jenshnielsen) wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #7961 +/- ##
==========================================
+ Coverage 70.08% 70.24% +0.15%
==========================================
Files 333 333 Lines 32250 32250 ==========================================
+ Hits 22602 22653 +51 + Misses 9648 9597 -51 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds new pytest coverage for several low-/uncovered QCoDeS utility, dataset, and metadatable modules, primarily validating type aliases, small helpers, and export/snapshot utilities.
Changes:
- Introduces new unit tests for
qcodes.utilshelpers (types, numpy/path utilities, deep update, abstractmethod, deprecate). - Adds dataset-focused tests (snapshot diffing, JSON exporter, export config, RunDescriber TypedDict schemas, SQLite settings).
- Adds metadatable base tests to cover previously unexercised branches.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/test_types.py | Tests NumPy type tuple definitions/composition in qcodes.utils.types. |
| tests/utils/test_abstractmethod.py | Tests qcodes_abstractmethod decorator behavior. |
| tests/utils/test_deprecate.py | Tests QCoDeSDeprecationWarning behavior and visibility. |
| tests/utils/test_deep_update_utils.py | Tests recursive dict merge semantics of deep_update. |
| tests/utils/test_path_helpers.py | Tests QCoDeS path helper utilities and env var behavior. |
| tests/utils/test_numpy_utils.py | Tests ragged/uniform array conversion helper. |
| tests/dataset/test_snapshot_utils.py | Tests dataset snapshot diffing (diff_param_snapshots). |
| tests/dataset/test_json_exporter.py | Tests JSON linear/heatmap export utilities and templates. |
| tests/dataset/test_export_config.py | Tests export config get/set helpers and enum mapping. |
| tests/dataset/test_rundescribertypes.py | Tests TypedDict schemas/unions for RunDescriber versioning. |
| tests/dataset/test_sqlite_settings_extended.py | Adds broader coverage for SQLite settings/limits helpers. |
| tests/test_metadatable_base.py | Adds coverage for Metadatable/MetadatableWithName behavior. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
b9bbb6d to
6f67291Compare…modules Add 128 new tests across 12 new test files targeting modules that previously had 0% or very low test coverage (excluding instrument_drivers). New test files: - tests/utils/test_types.py: numpy type tuple definitions and composition - tests/utils/test_abstractmethod.py: qcodes_abstractmethod decorator - tests/utils/test_deprecate.py: QCoDeSDeprecationWarning class - tests/utils/test_deep_update_utils.py: recursive dict merging - tests/utils/test_path_helpers.py: QCoDeS path resolution utilities - tests/utils/test_numpy_utils.py: ragged array conversion - tests/dataset/test_snapshot_utils.py: dataset snapshot diffing - tests/dataset/test_json_exporter.py: JSON linear/heatmap export - tests/dataset/test_export_config.py: export config get/set functions - tests/dataset/test_rundescribertypes.py: TypedDict versioned schemas - tests/dataset/test_sqlite_settings_extended.py: SQLite settings/limits - tests/test_metadatable_base.py: Metadatable and MetadatableWithName Non-driver test coverage: 64.7% -> 65.4% Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6f67291 to
302867aCompare| def test_data_export_type_enum_members() -> None: | ||
| assert DataExportType.NETCDF.value == "nc" | ||
| assert DataExportType.CSV.value == "csv" | ||
| assert len(DataExportType) == 2 | ||
| def test_get_data_export_type_with_string_netcdf() -> None: | ||
| result = get_data_export_type("NETCDF") | ||
| assert result is DataExportType.NETCDF | ||
| def test_get_data_export_type_with_string_csv() -> None: | ||
| result = get_data_export_type("CSV") | ||
| assert result is DataExportType.CSV |
There was a problem hiding this comment.
these are unneessary imho
| deftest_data_export_type_enum_members() ->None: | |
| assertDataExportType.NETCDF.value=="nc" | |
| assertDataExportType.CSV.value=="csv" | |
| assertlen(DataExportType) ==2 | |
| deftest_get_data_export_type_with_string_netcdf() ->None: | |
| result=get_data_export_type("NETCDF") | |
| assertresultisDataExportType.NETCDF | |
| deftest_get_data_export_type_with_string_csv() ->None: | |
| result=get_data_export_type("CSV") | |
| assertresultisDataExportType.CSV |
| def test_json_template_linear_structure() -> None: | ||
| assert json_template_linear["type"] == "linear" | ||
| assert "x" in json_template_linear | ||
| assert "y" in json_template_linear | ||
| assert isinstance(json_template_linear["x"], dict) | ||
| assert isinstance(json_template_linear["y"], dict) | ||
| assert "data" in json_template_linear["x"] | ||
| assert "data" in json_template_linear["y"] | ||
| assert json_template_linear["x"]["is_setpoint"] is True | ||
| assert json_template_linear["y"]["is_setpoint"] is False | ||
| def test_json_template_heatmap_structure() -> None: | ||
| assert json_template_heatmap["type"] == "heatmap" | ||
| assert "x" in json_template_heatmap | ||
| assert "y" in json_template_heatmap | ||
| assert "z" in json_template_heatmap | ||
| assert isinstance(json_template_heatmap["x"], dict) | ||
| assert isinstance(json_template_heatmap["y"], dict) | ||
| assert isinstance(json_template_heatmap["z"], dict) | ||
| assert json_template_heatmap["x"]["is_setpoint"] is True | ||
| assert json_template_heatmap["y"]["is_setpoint"] is True | ||
| assert json_template_heatmap["z"]["is_setpoint"] is False |
There was a problem hiding this comment.
not sure there's value in these imho
| deftest_json_template_linear_structure() ->None: | |
| assertjson_template_linear["type"] =="linear" | |
| assert"x"injson_template_linear | |
| assert"y"injson_template_linear | |
| assertisinstance(json_template_linear["x"], dict) | |
| assertisinstance(json_template_linear["y"], dict) | |
| assert"data"injson_template_linear["x"] | |
| assert"data"injson_template_linear["y"] | |
| assertjson_template_linear["x"]["is_setpoint"] isTrue | |
| assertjson_template_linear["y"]["is_setpoint"] isFalse | |
| deftest_json_template_heatmap_structure() ->None: | |
| assertjson_template_heatmap["type"] =="heatmap" | |
| assert"x"injson_template_heatmap | |
| assert"y"injson_template_heatmap | |
| assert"z"injson_template_heatmap | |
| assertisinstance(json_template_heatmap["x"], dict) | |
| assertisinstance(json_template_heatmap["y"], dict) | |
| assertisinstance(json_template_heatmap["z"], dict) | |
| assertjson_template_heatmap["x"]["is_setpoint"] isTrue | |
| assertjson_template_heatmap["y"]["is_setpoint"] isTrue | |
| assertjson_template_heatmap["z"]["is_setpoint"] isFalse |
| # --------------- RunDescriberV1Dict --------------- | ||
| def test_v1_dict_instantiation() -> None: |
There was a problem hiding this comment.
what does this and above actually test? the fact that a dict with given structure in the test matches the type definition?
| def test_numpy_concrete_ints_contents() -> None: | ||
| """Test that numpy_concrete_ints contains the expected fixed-size int types.""" | ||
| expected = ( | ||
| np.int8, | ||
| np.int16, | ||
| np.int32, | ||
| np.int64, | ||
| np.uint8, | ||
| np.uint16, | ||
| np.uint32, | ||
| np.uint64, | ||
| ) | ||
| assert numpy_concrete_ints == expected | ||
| def test_numpy_concrete_ints_length() -> None: | ||
| """Test that numpy_concrete_ints has 8 types.""" | ||
| assert len(numpy_concrete_ints) == 8 | ||
| def test_numpy_c_ints_contents() -> None: | ||
| """Test that numpy_c_ints contains the expected C-compatible int types.""" | ||
| expected = ( | ||
| np.uintp, | ||
| np.uintc, | ||
| np.intp, | ||
| np.intc, | ||
| np.short, | ||
| np.byte, | ||
| np.ushort, | ||
| np.ubyte, | ||
| np.longlong, | ||
| np.ulonglong, | ||
| ) | ||
| assert numpy_c_ints == expected | ||
| def test_numpy_c_ints_length() -> None: | ||
| """Test that numpy_c_ints has 10 types.""" | ||
| assert len(numpy_c_ints) == 10 | ||
| def test_numpy_non_concrete_ints_instantiable_contents() -> None: | ||
| """Test that numpy_non_concrete_ints_instantiable contains default int types.""" | ||
| expected = (np.int_, np.uint) | ||
| assert numpy_non_concrete_ints_instantiable == expected | ||
| def test_numpy_ints_is_combination() -> None: | ||
| """Test that numpy_ints is the concatenation of all int sub-tuples.""" | ||
| expected = numpy_concrete_ints + numpy_c_ints + numpy_non_concrete_ints_instantiable | ||
| assert numpy_ints == expected | ||
| def test_numpy_ints_length() -> None: | ||
| """Test that numpy_ints has the combined length of all int sub-tuples.""" | ||
| expected_len = ( | ||
| len(numpy_concrete_ints) | ||
| + len(numpy_c_ints) | ||
| + len(numpy_non_concrete_ints_instantiable) | ||
| ) | ||
| assert len(numpy_ints) == expected_len | ||
| def test_numpy_concrete_floats_contents() -> None: | ||
| """Test that numpy_concrete_floats contains fixed-size float types.""" | ||
| expected = (np.float16, np.float32, np.float64) | ||
| assert numpy_concrete_floats == expected | ||
| def test_numpy_c_floats_contents() -> None: | ||
| """Test that numpy_c_floats contains C-compatible float types.""" | ||
| expected = (np.half, np.single, np.double) | ||
| assert numpy_c_floats == expected | ||
| def test_numpy_floats_is_combination() -> None: | ||
| """Test that numpy_floats is the concatenation of float sub-tuples.""" | ||
| assert numpy_floats == numpy_concrete_floats + numpy_c_floats | ||
| def test_numpy_floats_length() -> None: | ||
| """Test that numpy_floats has the combined length of float sub-tuples.""" | ||
| assert len(numpy_floats) == len(numpy_concrete_floats) + len(numpy_c_floats) | ||
| def test_numpy_concrete_complex_contents() -> None: | ||
| """Test that numpy_concrete_complex contains fixed-size complex types.""" | ||
| expected = (np.complex64, np.complex128) | ||
| assert numpy_concrete_complex == expected | ||
| def test_numpy_c_complex_contents() -> None: | ||
| """Test that numpy_c_complex contains C-compatible complex types.""" | ||
| expected = (np.csingle, np.cdouble) | ||
| assert numpy_c_complex == expected | ||
| def test_numpy_complex_is_combination() -> None: | ||
| """Test that numpy_complex is the concatenation of complex sub-tuples.""" | ||
| assert numpy_complex == numpy_concrete_complex + numpy_c_complex |
There was a problem hiding this comment.
i think these don't add value because they just replicate the implementation, and sometimes encode implementation details that I don't think are relevant (e.g. how different type sets are composed). let's remove?
| deftest_numpy_concrete_ints_contents() ->None: | |
| """Test that numpy_concrete_ints contains the expected fixed-size int types.""" | |
| expected= ( | |
| np.int8, | |
| np.int16, | |
| np.int32, | |
| np.int64, | |
| np.uint8, | |
| np.uint16, | |
| np.uint32, | |
| np.uint64, | |
| ) | |
| assertnumpy_concrete_ints==expected | |
| deftest_numpy_concrete_ints_length() ->None: | |
| """Test that numpy_concrete_ints has 8 types.""" | |
| assertlen(numpy_concrete_ints) ==8 | |
| deftest_numpy_c_ints_contents() ->None: | |
| """Test that numpy_c_ints contains the expected C-compatible int types.""" | |
| expected= ( | |
| np.uintp, | |
| np.uintc, | |
| np.intp, | |
| np.intc, | |
| np.short, | |
| np.byte, | |
| np.ushort, | |
| np.ubyte, | |
| np.longlong, | |
| np.ulonglong, | |
| ) | |
| assertnumpy_c_ints==expected | |
| deftest_numpy_c_ints_length() ->None: | |
| """Test that numpy_c_ints has 10 types.""" | |
| assertlen(numpy_c_ints) ==10 | |
| deftest_numpy_non_concrete_ints_instantiable_contents() ->None: | |
| """Test that numpy_non_concrete_ints_instantiable contains default int types.""" | |
| expected= (np.int_, np.uint) | |
| assertnumpy_non_concrete_ints_instantiable==expected | |
| deftest_numpy_ints_is_combination() ->None: | |
| """Test that numpy_ints is the concatenation of all int sub-tuples.""" | |
| expected=numpy_concrete_ints+numpy_c_ints+numpy_non_concrete_ints_instantiable | |
| assertnumpy_ints==expected | |
| deftest_numpy_ints_length() ->None: | |
| """Test that numpy_ints has the combined length of all int sub-tuples.""" | |
| expected_len= ( | |
| len(numpy_concrete_ints) | |
| +len(numpy_c_ints) | |
| +len(numpy_non_concrete_ints_instantiable) | |
| ) | |
| assertlen(numpy_ints) ==expected_len | |
| deftest_numpy_concrete_floats_contents() ->None: | |
| """Test that numpy_concrete_floats contains fixed-size float types.""" | |
| expected= (np.float16, np.float32, np.float64) | |
| assertnumpy_concrete_floats==expected | |
| deftest_numpy_c_floats_contents() ->None: | |
| """Test that numpy_c_floats contains C-compatible float types.""" | |
| expected= (np.half, np.single, np.double) | |
| assertnumpy_c_floats==expected | |
| deftest_numpy_floats_is_combination() ->None: | |
| """Test that numpy_floats is the concatenation of float sub-tuples.""" | |
| assertnumpy_floats==numpy_concrete_floats+numpy_c_floats | |
| deftest_numpy_floats_length() ->None: | |
| """Test that numpy_floats has the combined length of float sub-tuples.""" | |
| assertlen(numpy_floats) ==len(numpy_concrete_floats) +len(numpy_c_floats) | |
| deftest_numpy_concrete_complex_contents() ->None: | |
| """Test that numpy_concrete_complex contains fixed-size complex types.""" | |
| expected= (np.complex64, np.complex128) | |
| assertnumpy_concrete_complex==expected | |
| deftest_numpy_c_complex_contents() ->None: | |
| """Test that numpy_c_complex contains C-compatible complex types.""" | |
| expected= (np.csingle, np.cdouble) | |
| assertnumpy_c_complex==expected | |
| deftest_numpy_complex_is_combination() ->None: | |
| """Test that numpy_complex is the concatenation of complex sub-tuples.""" | |
| assertnumpy_complex==numpy_concrete_complex+numpy_c_complex |
| @@ -0,0 +1,145 @@ | |||
| """ | |||
| Tests for qcodes.metadatable.metadatable_base covering branches | |||
| not exercised by test_metadata.py. | |||
There was a problem hiding this comment.
i'd add these to the test_metadata.py
| def test_metadatable_with_name_has_abstract_methods() -> None: | ||
| # MetadatableWithName uses @abstractmethod for static analysis; | ||
| # verify the property descriptors are marked abstract. | ||
| for attr_name in ("short_name", "full_name"): | ||
| descriptor = getattr(MetadatableWithName, attr_name) | ||
| assert isinstance(descriptor, property) | ||
| assert getattr(descriptor.fget, "__isabstractmethod__", False) |
There was a problem hiding this comment.
imho not valuable test as it duplicates implementation
| deftest_metadatable_with_name_has_abstract_methods() ->None: | |
| # MetadatableWithName uses @abstractmethod for static analysis; | |
| # verify the property descriptors are marked abstract. | |
| forattr_namein ("short_name", "full_name"): | |
| descriptor=getattr(MetadatableWithName, attr_name) | |
| assertisinstance(descriptor, property) | |
| assertgetattr(descriptor.fget, "__isabstractmethod__", False) |
…modules
Add 128 new tests across 12 new test files targeting modules that previously had 0% or very low test coverage (excluding instrument_drivers).
New test files:
Coverage: from 69.97% to 70.12% +0.15%