Stronger, clearer test suite with cross-engine verification#195
Merged
Conversation
Add an autouse fixture that snapshots and restores Logger._reaction so the WARN mode set by _warn() cannot leak into later raise-expecting tests (e.g. core/test_guard, core/test_parameters).
Add and extend supplementary coverage tests for analysis (analysis, fitting, minimizers/base), display (plotting, progress), report (data_context, html_renderer), background estimate, and the package __main__ entry point. Raises overall unit coverage to ~84.5%.
Supplementary unit tests lift tex_renderer 67->100%, iucr_writer 84->97%, cif serialize 83->96%, and fit tracking 78->98%. Overall unit coverage rises to ~86.3%.
Supplementary unit tests lift project/display 73->100%, utils/logging 76->99%, utils/utils 86->99%, and experiment item/base 76->99%. Overall unit coverage rises to ~87.8%.
Supplementary unit tests lift display/plotting 82->93%, analysis 82->94%, plotters/plotly 78->97%, and project/project 80->100%. Overall unit coverage crosses 90% (90.0%).
Supplementary unit tests lift analysis/sequential 53->99%, utils/environment 69->95%, io/results_sidecar 81->99%, and experiment collection 47->100%. Overall unit coverage rises to ~91.4%.
Document previously-undocumented methods, properties, setters, and __init__s across analysis (categories, minimizers, fit_helpers, calculators), datablocks/experiment (peak, instrument), and display. Raises docstring coverage from 86.2% to 94.4% (interrogate).
_plot_single_crystal_posterior_predictive_summary called PlotlyPlotter._get_diagonal_shape() with no arguments, but the method requires (minimum, maximum); the reflection-check plot raised TypeError whenever reached. Compute the shared axis range via single_crystal_axis_range (matching build_single_crystal_figure) and pass it through. Adds a test covering the now-working build path.
test_relative_destination_does_not_depend_on_current_project relied on a sibling test leaving EASYDIFFRACTION_ARTIFACT_ROOT cleared; it failed in isolation because the configured artifact root rooted the relative destination under tmp/tutorials instead of the cwd. Clear the env var in the test so the cwd-resolution branch is exercised deterministically.
Add pytest-randomly so unit tests run in randomised order (they ran single-process in deterministic alphabetical order, which hid ordering bugs). Add a tests/unit autouse fixture that snapshots and restores the Logger reaction/mode and the environment around every test, enforcing a clean RAISE baseline so no test can leak global state forward. Raise the coverage floor from 80% to 88% (currently 91.4%).
test_extinction_model_invalid (expects invalid input to fall back and keep the current value) and Structures test_show_params (expects the None-not-callable TypeError) only passed when an alphabetically-earlier test had leaked Logger WARN mode. Set WARN explicitly in each so they pass deterministically under randomised order.
Record issue 116 — adding mypy/pyright/ty would have caught the wrong-arity _get_diagonal_shape call at lint time. Notes an incremental rollout to manage the initial backlog.
Verify-review-1 finding 1: pytest-randomly was not in the plan's pre-approval list. It was approved by direct user request in the conversation (AGENTS.md §Architecture first approval path), so it need not be removed; record that approval in the plan's Decisions section for traceability.
On feature-branch pushes the tier selection is 'not pr and not
nightly'. Every integration test is auto-marked pr, so the selection is
empty and pytest exits 5 ('no tests collected'), failing the step under
set -euo pipefail. Add a run-pr-tier output from env-prepare and gate
both integration-test steps (source-test and package-test) on it, so
integration runs only in the pr tier (PRs + develop/master) as designed
and is cleanly skipped otherwise.
test_print_path_is_stringified hardcoded a POSIX path; str(Path) uses backslashes on Windows. Compare against str(Path(...)) instead. test_read_posterior_payload_reads_aux_arrays intermittently hit FileNotFoundError creating an HDF5 file in a fresh tmp dir on Windows CI (all sibling h5py tests share the pattern and passed, so this is an environmental flake). Force the parent dir to exist and pass a str path to remove the deterministic vectors.
The low-level sidecar helpers (_create_dataset, _read_dataset, _read_payload_group, _write_payload_group, _read_posterior_payload) take an open h5py handle and don't care whether it is disk-backed. Writing real .h5 files to tmp_path dragged the platform filesystem into these unit tests and caused an intermittent Windows ENOENT on file creation. Add an _in_memory_h5() context manager using h5py's core driver (backing_store=False) and route the 10 low-level tests through it: no disk, no paths, hermetic, faster, and immune to FS flakiness on every platform. Real on-disk tmp_path files remain only for the two public-API tests that read a sidecar from a directory. Replaces the mkdir+str band-aid; module coverage unchanged at 99%.
test_display_path_falls_back_to_absolute monkeypatched pathlib.Path.relative_to to raise unconditionally, to simulate the different-Windows-drive case (which POSIX cannot reach with walk_up). That global patch also broke pytest's own relative_to call in its result reporter (cwd_relative_nodeid -> bestrelpath), which only runs when the invocation dir differs from the rootdir -- i.e. the package-test job (pytest ../tests/unit from a subproject), crashing it with INTERNALERROR 'on a different drive'. Make the fake raise only for the resolved target path and delegate every other call, so pytest internals keep working while the except-ValueError fallback is still exercised cross-platform.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #195 +/- ##
============================================
+ Coverage 80.24% 90.89% +10.64%
============================================
Files 263 264 +1
Lines 22210 22225 +15
Branches 2587 2587
============================================
+ Hits 17823 20201 +2378
+ Misses 3411 1507 -1904
+ Partials 976 517 -459
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
AndrewSazonov
added
the
[scope] maintenance
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.