Add regression tests for the critical-review bug fixes - #76
Merged
Conversation
Adds 128 new tests (93 -> 221 passing) across 7 files, one per area from the original critical review, each targeting the specific silent-corruption/security bugs found and fixed there rather than general coverage: - test-io-corruption-fixes.R: bruker.R/import_jdx.R parsing off-by-ones, missing-field validation, and the nmr_read_bruker_fid rewrite (acqus-driven byte order/dtype/TD/SW_h, data-frame return). - test-nmr-meta-fixes.R: nmr_meta_add's multi-column join-key truncation, nmr_meta_get's unknown-groups validation, and nmr_meta_export's warning message formatting. - test-area-estimation-fixes.R: get_norm_rmse's argument-collision fix (including a body-introspection guard on the call site), get_peak_bounds's NA sentinel, refine_lorentzian_fit_with_nls's error capture, and peaklist_fit_lorentzians's error accumulation. - test-signal-processing-fixes.R: nmr_autophase's absorptionOnly passthrough (verified via a mocked NMRphasing binding) and message truncation, nmr_normalize's divide-by-zero/non-positive-factor guards and method="value" validation, nmr_interpolate's out-of-range warning. - test-stats-fixes.R: bp_VIP_analysis's permutation fix and seq_along fix (functional tests plus source-introspection backstops), and the PDF device-leak fix in plsda.R's plotting functions (forced via a mocked mixOmics::plotIndiv failure). - test-plots-rdolphin-fixes.R: plots.R's silent-subsampling notice, to_rDolphin_blood.R's label-encoding message. - test-security-fixes.R: the zip-slip guard in download_MTBLS242 (against a genuinely constructed malicious zip, network mocked out) and nmr_dataset_load's class validation. No R/ source files were changed; all fixes were already merged in prior PRs. Full suite: 221 passed, 0 failed, 1 pre-existing skip (rDolphin not installed).
The previous commit added 7 standalone "-fixes" files grouped by review batch. Moved each test into the existing (or a newly created, correspondingly-named) test file for the R/ source file it actually covers, and reworded test names/comments to describe expected behavior directly instead of referencing "the critical review" or "previously had a bug" - a fix's own history isn't relevant once it's just normal test coverage. - bruker.R/import_jdx.R tests -> test-read_bruker.R / test-jdx.R - nmr_meta.R tests -> test_adding_metadata.R - area_estimation.R tests -> test-peak-fitting.R - nmr_autophase.R/nmr_normalize.R/nmr_interpolate.R tests -> test-autophase.R / test-normalization.R / test_interpolation.R - nmr_data_analysis.R (bp_VIP_analysis) tests -> test-nmr-data-analysis.R - plsda.R tests -> new test-plsda.R (no prior dedicated file) - plots.R tests -> new test-plots.R (no prior dedicated file) - to_rDolphin_blood.R test -> test_rDolphin.R - download_MTBLS242.R tests -> new test-download_MTBLS242.R - nmr_dataset_load_save.R tests -> new test-nmr_dataset_load_save.R All 221 tests still pass (same count as before the reorg) - no assertions were added, removed, or weakened, only relocated/reframed.
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
2 participants
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.
Summary
Adds 128 new tests (93 → 221 passing), each targeting a specific silent-data-corruption/security bug fixed during a prior critical review, rather than general/blanket coverage. Tests live in the existing (or newly created, correspondingly-named) test file for the
R/source file they cover — not grouped by when/why they were written:test-read_bruker.R—read_orig_file()'s single-token-line handling,read_bin_data()'s file-open error reporting,nmr_read_bruker_fid()'s acqus-driven byte order/dtype/TD/SW_hparsing and data-frame return.test-jdx.R—process_block()'s single-token data-line handling andXUNITS/.OBSERVE FREQUENCYvalidation.test_adding_metadata.R—nmr_meta_add()'s multi-column join-key handling,nmr_meta_get()'s unknown-groups=validation,nmr_meta_export()'s warning-message formatting.test-peak-fitting.R—get_norm_rmse()'s formula contract (plus a body-introspection guard on its call site),get_peak_bounds()'sNA-on-missing-inflection-point behavior,refine_lorentzian_fit_with_nls()'s error capture,peaklist_fit_lorentzians()'s error accumulation.test-autophase.R—nmr_autophase()'sabsorptionOnlypassthrough (verified by mockingNMRphasing::NMRphasing) and missing-imaginary-data message summarization.test-normalization.R—nmr_normalize()/norm_pqn()'s divide-by-zero and non-positive-factor guards,method="value"validation.test_interpolation.R—nmr_interpolate_1D()'s out-of-native-range warning.test-nmr-data-analysis.R—bp_VIP_analysis()'s permutation behavior and single-class-resample recovery (functional tests plus source-introspection invariant checks).test-plsda.R(new) —plot_plsda_samples()/plot_plsda_multimodel()'s graphics-device cleanup, verified by forcing amixOmics::plotIndivfailure.test-plots.R(new) —plot()'s >20-sample subsampling notice.test_rDolphin.R—files_to_rDolphin()'s label-to-numeric-code mapping message.test-download_MTBLS242.R(new) — the zip-slip guard, tested against a genuinely constructed malicious zip archive (network access mocked out, no real downloads).test-nmr_dataset_load_save.R(new) —nmr_dataset_load()'s class validation.No
R/source files were changed — all the underlying fixes were already merged in prior PRs (#71, #73).Test plan
devtools::test()→[ FAIL 0 | WARN 1 | SKIP 1 | PASS 221 ]rDolphinnot installed in this environment)R/source files were modified (tests-only diff)