Integrate two old_to_integrate tests; harden test discovery (#80)

* Integrate two old_to_integrate tests; harden test discovery

Convert two quarantined standalone scripts into real matlab.unittest tests
and move them into the discovered suite, and make the runner robust against
non-test files that match the canlab_test_*.m glob.

- image_vector/canlab_test_check_roi_extraction.m: converts
  old_to_integrate/check_roi_extraction.m. The original printed PASS/FAIL but
  never asserted. Now asserts multi-region 'unique_mask_values' extraction and
  invariance of the region averages across a write->reload round-trip
  (RelTol 1e-3 / AbsTol 1e-2 absorbs single-precision NIfTI rounding).
  Complements canlab_test_extract_roi (single-mask only).
- image_vector/canlab_test_resampling_pattern_expression.m: converts
  old_to_integrate/resampling_pattern_expression_unit_test1.m, which only
  plotted. Now asserts SIIPS pattern expression is stable (corr > 0.99) across
  default/nearest/spline resampling; skips if SIIPS is unavailable.
- helpers/canlab_safe_suite_from_file.m + canlab_run_all_tests.m: wrap
  TestSuite.fromFile so a stray non-test canlab_test_*.m is warn-skipped
  instead of throwing NonTestFile and aborting discovery of the whole suite.
- canlab_test_runner_robustness.m: pins that behavior (non-test file ->
  warn + empty + ok=false; valid file -> loads).

Old originals removed from old_to_integrate (jackknife_similarity_unit_test.m
stays - it is a characterization/figure script, not a unit test).

Full default suite after: 147 passed, 0 failed, 2 incomplete (pre-existing
environment skips).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Convert 5 xval_* tests to matlab.unittest; move to predictive_model/

The five xval_*_unit_test.m files at the Unit_tests root were genuine tests
(real assertions, no graphics, deterministic, bundled DPSP / synthetic data)
but were plain-assert functions named outside the canlab_test_* convention,
so the runner never discovered them.

Convert each into a functiontests file under a new predictive_model/ subdir:
the model is fit once in setupOnce (skipped via assumeTrue if the DPSP sample
data is absent) and cached, and the assertions are split into focused test
functions using tc.verify* for granular reporting. Behavior preserved.

- predictive_model/canlab_test_xval_SVM.m
- predictive_model/canlab_test_xval_SVR.m
- predictive_model/canlab_test_xval_regression_multisubject.m
- predictive_model/canlab_test_xval_regression_multisubject_featureselect.m
- predictive_model/canlab_test_xval_discriminant_classifier.m
- helpers/canlab_get_dpsp_hot_warm.m: shared DPSP Hot/Warm loader.

Old xval_*_unit_test.m removed. New suite: 25 test points, all pass, ~7.5s.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Convert predictive_model_unit_test to matlab.unittest (minimal nboot/nperm)

predictive_model_unit_test.m exercised the @predictive_model object's own
public API (fit/predict/score/crossval/bootstrap/weight_map_object/
permutation_test/calibrate/select_features/grid_search/stability_selection/
pipeline/newapi routing/regression+report/summary). This is non-redundant
with the xval_* tests, which cover the legacy wrapper functions rather than
the object methods - so it is worth keeping.

Convert to predictive_model/canlab_test_predictive_model_api.m: the shared
fit -> crossval -> bootstrap chain is computed once in setupOnce and cached;
the rest are focused test functions using tc.verify*; visualisation methods
(plot/confusionchart/montage) are smoke-tested and skipped on a headless
runner via canlab_classify_environment_error.

Drop the bootstrap/permutation/stability counts to minimum-viable values
(nboot 25->5, nperm 10->3, stability nboot 20->5) since this is a smoke test,
not real inference; assertions are structural (shapes/ranges/p-floor), so the
counts only need to exercise the code paths. Runtime 131s -> 73s locally; the
residual cost is the high-dimensional (194,676-feature) cross-validation, not
the resampling counts. Skipped if DPSP_hotwarm is not on the path.

Old predictive_model_unit_test.m removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* predictive_model API test: mask to sparse gray-matter ROI for speed

The dominant cost was cross-validating on the full ~195k-voxel volume. In
setupOnce, restrict hw_obj to gray matter (gray_matter_mask.img) and then
deterministically thin to every 8th voxel (~20k features). hw_obj and X stay
in lockstep, so weight_map_object / montage back-project consistently, and
all assertions are structural so masking does not change them. Falls back to
the full volume if the mask is not on the path.

Runtime: 131s (original) -> 73s (minimal nboot/nperm) -> ~20s now. All 17
test points still pass; model behaviour unchanged (AUC ~0.83).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: torwager

CanlabCore/Unit_tests/canlab_run_all_tests.m

@@ -63,7 +63,9 @@
         case 'include'
             % run everything
     end
-    suite = [suite, TestSuite.fromFile(fpath)]; %#ok<AGROW>
+    % canlab_safe_suite_from_file warn-skips files that are not valid test
+    % files instead of letting a NonTestFile error abort the whole run.
+    suite = [suite, canlab_safe_suite_from_file(fpath)]; %#ok<AGROW>
 end
 
 tag = char(p.Results.Tag);

CanlabCore/Unit_tests/canlab_test_runner_robustness.m

@@ -0,0 +1,76 @@
+function tests = canlab_test_runner_robustness
+%CANLAB_TEST_RUNNER_ROBUSTNESS Discovery must not abort on a stray non-test file.
+%
+% canlab_run_all_tests globs every canlab_test_*.m under Unit_tests and
+% concatenates the suites. A file that matches the name pattern but is not a
+% valid matlab.unittest test (an old plain-assert script, or a function whose
+% internal name does not match the filename) makes TestSuite.fromFile throw
+% MATLAB:unittest:TestSuite:NonTestFile. Without guarding, that single error
+% aborts discovery of the entire suite. canlab_safe_suite_from_file warn-skips
+% such files instead; these tests pin that behavior.
+
+tests = functiontests(localfunctions);
+end
+
+
+function test_nontest_file_is_warn_skipped(tc)   %#ok<*DEFNU>
+% A canlab_test_*.m whose function name != filename and that never calls
+% functiontests is not a valid test file: skip it, warn, return empty.
+folder = tc.applyFixture( ...
+    matlab.unittest.fixtures.TemporaryFolderFixture).Folder;
+fpath = local_write(folder, 'canlab_test_bogus_nontest.m', { ...
+    'function some_other_name()'
+    'assert(true);'
+    'end'});
+
+% Use lastwarn rather than verifyWarning so the check does not depend on the
+% ambient warning display state (a caller may have warnings off; lastwarn
+% still records the id even when the warning event is not displayed).
+lastwarn('', '');
+[suite, ok] = canlab_safe_suite_from_file(fpath);
+[~, warn_id] = lastwarn;
+
+tc.verifyFalse(ok, 'expected ok=false for a non-test file');
+tc.verifyEmpty(suite, 'expected an empty suite for a non-test file');
+tc.verifyEqual(warn_id, 'canlab_run_all_tests:skippedNonTestFile', ...
+    'expected a skippedNonTestFile warning');
+end
+
+
+function test_valid_test_file_is_loaded(tc)
+% A well-formed functiontests file loads normally (ok=true, non-empty suite).
+folder = tc.applyFixture( ...
+    matlab.unittest.fixtures.TemporaryFolderFixture).Folder;
+fpath = local_write(folder, 'canlab_test_valid_dummy.m', { ...
+    'function tests = canlab_test_valid_dummy'
+    'tests = functiontests(localfunctions);'
+    'end'
+    ''
+    'function test_trivial(tc)'
+    'tc.verifyTrue(true);'
+    'end'});
+
+[suite, ok] = canlab_safe_suite_from_file(fpath);
+tc.verifyTrue(ok, 'expected ok=true for a valid test file');
+tc.verifyNotEmpty(suite);
+tc.verifyEqual(numel(suite), 1);
+end
+
+
+function fpath = local_write(folder, name, lines)
+% Write a cellstr of lines to folder/name and return the full path. Uses
+% fopen/fprintf (not writelines) so the test runs on older MATLAB too.
+fpath = fullfile(folder, name);
+fid = fopen(fpath, 'w');
+tc_assert_open(fid, fpath);
+cleanup = onCleanup(@() fclose(fid));
+fprintf(fid, '%s\n', lines{:});
+end
+
+
+function tc_assert_open(fid, fpath)
+if fid < 0
+    error('canlab_test_runner_robustness:cannotWrite', ...
+        'Could not open %s for writing.', fpath);
+end
+end

CanlabCore/Unit_tests/helpers/canlab_get_dpsp_hot_warm.m

@@ -0,0 +1,27 @@
+function [hot, warm, ok] = canlab_get_dpsp_hot_warm()
+%CANLAB_GET_DPSP_HOT_WARM Load the DPSP single-subject Hot/Warm sample maps.
+%
+%   [hot, warm, ok] = canlab_get_dpsp_hot_warm()
+%
+%   Returns the single-subject Hot and Warm condition maps from
+%   Sample_datasets/DPSP_pain_rejection_participant_maps as fmri_data objects.
+%   Used by the predictive_model / xval_* tests so the load semantics live in
+%   one place. ok is false (and hot/warm are []) if the sample files are not
+%   on the path, so callers can assume/skip gracefully.
+
+sample_dir = fullfile(fileparts(fileparts(which('fmri_data'))), ...
+    'Sample_datasets', 'DPSP_pain_rejection_participant_maps');
+hot_file  = fullfile(sample_dir, 'DPSP_single_subject_images_hot.mat');
+warm_file = fullfile(sample_dir, 'DPSP_single_subject_images_warm.mat');
+
+ok = exist(hot_file, 'file') == 2 && exist(warm_file, 'file') == 2;
+hot = [];
+warm = [];
+
+if ok
+    H = load(hot_file);
+    W = load(warm_file);
+    hot  = H.single_subject_images_hot;
+    warm = W.single_subject_images_warm;
+end
+end

CanlabCore/Unit_tests/helpers/canlab_safe_suite_from_file.m

@@ -0,0 +1,42 @@
+function [suite, ok] = canlab_safe_suite_from_file(fpath)
+%CANLAB_SAFE_SUITE_FROM_FILE Build a test suite from one file without aborting on bad files.
+%
+%   [suite, ok] = canlab_safe_suite_from_file(fpath)
+%
+%   Wraps matlab.unittest.TestSuite.fromFile so that a file which is not a
+%   valid test (e.g. a stray canlab_test_*.m that is an old plain-assert
+%   script or whose internal function name does not match the filename)
+%   does not throw and abort discovery of the whole suite. Such a file is
+%   warn-skipped and an empty Test array is returned instead.
+%
+%   This matters because canlab_run_all_tests globs every canlab_test_*.m
+%   under Unit_tests and concatenates the results; a single NonTestFile
+%   error from fromFile would otherwise take down the entire run.
+%
+% :Inputs:
+%   **fpath:**  absolute path to a candidate .m test file.
+%
+% :Outputs:
+%   **suite:**  a matlab.unittest.Test array (empty if the file is not a
+%               valid test file).
+%   **ok:**     logical, true if fromFile succeeded, false if the file was
+%               skipped.
+%
+% :See also: canlab_run_all_tests, matlab.unittest.TestSuite
+
+ok = true;
+suite = matlab.unittest.Test.empty;
+
+try
+    suite = matlab.unittest.TestSuite.fromFile(fpath);
+catch ME
+    ok = false;
+    if strcmp(ME.identifier, 'MATLAB:unittest:TestSuite:NonTestFile')
+        warning('canlab_run_all_tests:skippedNonTestFile', ...
+            'Skipping %s: not a valid matlab.unittest test file.', fpath);
+    else
+        warning('canlab_run_all_tests:fromFileError', ...
+            'Skipping %s: %s (%s)', fpath, ME.message, ME.identifier);
+    end
+end
+end

CanlabCore/Unit_tests/image_vector/canlab_test_check_roi_extraction.m

@@ -0,0 +1,58 @@
+function tests = canlab_test_check_roi_extraction
+%CANLAB_TEST_CHECK_ROI_EXTRACTION Multi-region ROI extraction + write/reload round-trip.
+%
+% Complements canlab_test_extract_roi (single-mask extraction) by exercising
+% the two things that test does not:
+%   1. Multi-region extraction with 'unique_mask_values' (one average per
+%      integer label in a parcellation), using atlas_labels_combined.img.
+%   2. Invariance of the extracted region averages across a write-to-disk and
+%      reload cycle - a regression guard on the NIfTI I/O path.
+%
+% Converted from the old standalone script Unit_tests/old_to_integrate/
+% check_roi_extraction.m, which printed PASS/FAIL but never asserted.
+
+tests = functiontests(localfunctions);
+end
+
+
+function test_unique_mask_values_roundtrip(tc)   %#ok<*DEFNU>
+mask_file = which('atlas_labels_combined.img');
+tc.assumeNotEmpty(mask_file, 'atlas_labels_combined.img not on path');
+
+mask_image = fmri_data(mask_file, 'noverbose');
+
+% Synthetic per-region timeseries: region i carries the signal ts*sqrt(i),
+% so every region has a distinct, known mean trajectory across images.
+wh_region = mask_image.dat;
+regions   = unique(wh_region);
+nimgs     = 20;
+ts        = linspace(-10, 10, nimgs);
+
+dat = mask_image;
+dat.dat = zeros(size(mask_image.dat, 1), nimgs);
+for i = 1:numel(regions)
+    idx = wh_region == regions(i);
+    dat.dat(idx, :) = repmat(ts .* sqrt(i), sum(idx), 1);
+end
+
+cl1      = extract_roi_averages(dat, mask_image, 'unique_mask_values');
+all_reg1 = cat(2, cl1(:).dat);
+
+% One row per image, one column per non-zero region label.
+tc.verifyEqual(size(all_reg1, 1), nimgs);
+tc.verifyGreaterThan(size(all_reg1, 2), 1);
+
+% Round-trip through disk in a scratch folder, then re-extract.
+tc.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture);
+dat.fullpath = fullfile(pwd, 'test_roi_image.nii');
+write(dat, 'overwrite');
+reloaded = fmri_data(dat.fullpath, 'noverbose');
+
+cl2      = extract_roi_averages(reloaded, mask_image, 'unique_mask_values');
+all_reg2 = cat(2, cl2(:).dat);
+
+tc.verifyEqual(size(all_reg2), size(all_reg1));
+% Small absolute slack absorbs single-precision NIfTI write rounding (the
+% original script used a 1e-3 relative threshold for the same reason).
+tc.verifyEqual(all_reg2, all_reg1, 'AbsTol', 1e-2, 'RelTol', 1e-3);
+end

CanlabCore/Unit_tests/image_vector/canlab_test_resampling_pattern_expression.m

@@ -0,0 +1,48 @@
+function tests = canlab_test_resampling_pattern_expression
+%CANLAB_TEST_RESAMPLING_PATTERN_EXPRESSION Pattern expression is stable across resampling.
+%
+% apply_mask(..., 'pattern_expression') resamples the weight map into the
+% data space when the two differ. This test checks that the resulting
+% pattern-expression scores barely change regardless of which interpolation
+% method does that resampling (default vs nearest vs spline) - i.e. the
+% dot-product readout is not an artifact of the interpolation choice.
+%
+% Uses the SIIPS signature against the emotionreg sample. Skipped if the
+% SIIPS image set is not available on the path (it ships with
+% Neuroimaging_Pattern_Masks).
+%
+% Converted from the old standalone visual script
+% Unit_tests/old_to_integrate/resampling_pattern_expression_unit_test1.m,
+% which only plotted the variants and asserted nothing.
+
+tests = functiontests(localfunctions);
+end
+
+
+function test_pattern_expression_stable_across_interp(tc)   %#ok<*DEFNU>
+obj = canlab_get_sample_fmri_data();
+
+try
+    siips = load_image_set('siips', 'noverbose');
+catch ME
+    tc.assumeFail(['SIIPS signature not available on this runner: ' ME.message]);
+end
+tc.assumeNotEmpty(siips.dat, 'SIIPS loaded but empty');
+
+n = size(obj.dat, 2);
+
+% Default path lets apply_mask resample internally; the other two pre-resample
+% the weight map with an explicit interpolation method.
+pe_default = apply_mask(obj, siips, 'pattern_expression');
+pe_nearest = apply_mask(obj, resample_space(siips, obj, 'nearest'), 'pattern_expression');
+pe_spline  = apply_mask(obj, resample_space(siips, obj, 'spline'),  'pattern_expression');
+
+for v = {pe_default, pe_nearest, pe_spline}
+    tc.verifyEqual(numel(v{1}), n);
+    tc.verifyTrue(all(isfinite(v{1})), 'pattern expression returned non-finite values');
+end
+
+% Empirically these correlate at >= 0.9999; 0.99 is a safe regression floor.
+tc.verifyGreaterThan(corr(pe_default, pe_nearest), 0.99);
+tc.verifyGreaterThan(corr(pe_default, pe_spline),  0.99);
+end