Skip to content

TEST fix or remove tests that could not fail, and run three that never ran - #128

Merged
jmschrei merged 16 commits into
mainfrom
test_fix_vacuous_checks
Oct 19, 2026
Merged

jmschrei merged 16 commits into
mainfrom
test_fix_vacuous_checks

Conversation

@jmschrei

@jmschrei jmschrei commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

This is the second of three test-only PRs from a file-by-file review of the suite. It fixes or removes tests that could not fail, and makes three tests that never ran start running. The next PR removes redundant tests and merges duplicates; the one after expands weak assertions.

How each change was checked. Every check added or tightened here was run against a copy of the library broken in the way the check guards, applied in a scratch copy or by monkeypatching, never in the repository. Each failed there and passes on the real code. Every removed test was read against the test said to cover it.

Tests that passed whatever the code did

  • test_match: test_extract_and_filter_chrom_signal_threshold tested thresholds 30 and 10.
    • At 30 every window passes, the same as with no bigWig. At 10 none do, so its second half asserted inside a loop over {}.
    • It now runs 10, 15, 21 and 30, and checks how many windows are kept, the empty case included.
    • The GC-window tests caught any error from extract_loci with a bare except: continue. The call is no longer wrapped.
  • test_saturation_mutagenesis:
    • test_saturation_mutagenesis_ordering compared at 2 decimals attributions that are all below 0.0025, so it passed against zeros. It now compares at 4.
    • test_saturation_mutagenesis_packed never reached the fast path, because a job this small skips it. It now forces the fast path and fails if it falls back.
  • test_deep_lift_shap:
    • test_deep_lift_shap_random_state compared 16 examples with 10, so it passed on the shape mismatch alone.
    • A missing comma made the numpy-multipliers case of test_hypothetical_attributions_raises repeat another case. The "Shape of all tensors must match." branch was not tested; it now is.
    • The cleanup-on-error tests raised on target=5 before any backward hook ran, so they passed with a cleanup that only removes handles. The hook test now raises inside the softmax rule, and both check every attribute _clear_hooks removes.
    • Three tests asserted the dtype of the input fixture instead of the attributions'.
  • test_pisa:
    • test_pisa_n_shuffles and three "tiny threshold warns" lines called deep_lift_shap instead of pisa.
    • In test_pisa_raises, the references came from a two-sequence fixture while X had sixteen sequences, so the channel and length lines failed on the batch axis. They are now built from the test's own X and matched to their shapes.
  • test_ersatz:
    • Three lines in each of the insert and substitute raise_ends tests passed an int as alphabet.
    • The raises_alphabet motifs had no batch axis, so they never reached the channel or length check.
    • test_local_dinucleotide_shuffle_bin_layout_valid checked a helper defined in the test file. It now checks the library's _local_bins over 200 seeds.
  • design:
    • test_beam_substitution_max_iter_default and _input_mask passed when no search step ran.
    • test_greedy_substitution_reverse_complement never set reverse_complement=True. It now compares that search with one over the motifs and their reverse complements listed explicitly.
  • test_plot: test_plot_logo_start_end_slicing passed with start and end ignored. It now compares the result with the logo of the sliced attributions.

Checks of pandas, figwig, torch, the toy models, or a call against itself

  • Removed tests:
    • test_load_signals_nan (figwig).
    • test_predict_raises_shape (the toy model's permute).
    • test_marginalize_additional_func_kwargs_none (the default passed explicitly).
    • test_marginalize_annotations_threads_kwargs: equal outputs whether or not the keywords are forwarded.
    • test_saturation_mutagenesis_no_warning_on_hard_one_hot: the same call as two other tests.
    • test_plot_logo_color_dict: only ax is not None, which _still_per_character covers.
    • test_transformer_block_attention_mask_forward (toy model only).
  • Removed lines and cases:
    • the df.__eq__ lines in the _interleave_loci tests (pandas);
    • the n_jobs=1 case of test_extract_loci_n_jobs;
    • the batch sizes that are clamped to the same single batch in test_dependency_map_batch_size and test_saliency_batch_size;
    • the torch Conv1d line of test_screen_raises.
  • Unused device fixture removed from tests that raise before anything moves. Their CUDA cases repeated the CPU ones.
  • deep_lift_shap rule tests: test_deep_lift_shap_rules_input_type (45 calls on identical inputs) and test_deep_lift_shap_rules_return_references (a NamedTuple field compared with itself) are removed. Their one real check each moves into test_deep_lift_shap_input_type and test_deep_lift_shap_return_references_named_tuple.
  • Attention mask: a forward pass now checks that the toy model's mask changes its output. This replaces eight deep_lift_shap calls.

Tests that never ran

  • Without BioPython. test_extract_loci_fasta_bgzip_without_biopython checks the error pyfaidx raises for a block-gzip FASTA without BioPython, and was skipped wherever BioPython was installed. pyfaidx imports BioPython only when it opens such a file, so the test now hides it through sys.modules and runs either way. Its counterpart, test_extract_loci_fasta_bgzip, still runs only where BioPython is installed, which CI does not install.
  • Console script. test_console_script_print_path is marked cmd, which addopts and CI deselect. Three new tests call main() in process instead: --print-path, --dest, and a repeated --dest with and without --force.
  • Skipped cases. Four cases of test_packed_sequence_from_one_hot_raises_values skipped. The parameters are now explicit (dtype, value) pairs.
  • Skipped test. test_deep_lift_shap_subclassed_op is now a strict xfail instead of a skip.

Checks

  • Full suite on the CPU and one CUDA device: 3,504 passed, 3 skipped, 2 xfailed. The skips are pisa's two existing CUDA TODOs and the block-gzip FASTA test, which needs BioPython.
  • sphinx -W passes.
  • prepush's credits check flags @RULE_PARAMS, a removed decorator, as a removed handle. Nothing was credited.

The six surfaces

  1. Tests: they are the change.
  2. Docs, README: not touched.
  3. Changelog: a Testing section under Unreleased.
  4. Bundled skill: not affected.
  5. Compatibility: no library code changes.

🤖 Generated with Claude Code

jmschrei and others added 16 commits October 19, 2026 15:07
…search able to fail

test_extract_and_filter_chrom_signal_threshold checked thresholds 30 and
10. At 30 every window of chr1 passes, as with no bigWig, and at 10 none
does, so the second half looped over an empty dict and asserted nothing;
its expected values fit a threshold of 15. It now runs 10, 15, 21 and 30
and checks the number of windows kept, the empty case included. The four
GC-window tests wrapped extract_loci in a bare except that skipped to the
next window, so a regression that made it raise left them with no
assertions; the calls now run bare.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tself

The df.__eq__ assertions in the _interleave_loci tests checked that
pandas refuses to compare frames with different labels, which each
test's own (df == expected).all(None) already implies.
test_load_signals_nan checked figwig's read of test3.bw; its tangermeme
assertions are in test_load_signals_bw, and the NaN-to-zero conversion is
checked by test_extract_loci_nans. The n_jobs=1 case of
test_extract_loci_n_jobs compared a call with itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Four of the twenty cases paired an integer dtype with 0.5 or NaN, which it
cannot hold, and skipped. The cases are now listed as dtype and value
pairs, the same sixteen that ran before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… installed

test_extract_loci_fasta_bgzip_without_biopython checks the ImportError
pyfaidx raises for a block-gzip FASTA without BioPython, and was skipped
whenever BioPython was installed. pyfaidx imports BioPython only when it
opens a block-gzip file, so the test now hides the module through
sys.modules and runs either way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_plot_logo_start_end_slicing asserted that the x range was at least
29 wide, which also held when start and end were ignored (it is 50 then).
It now compares the limits, glyph paths and colors with the logo of the
sliced attributions. test_plot_logo_color_dict only asserted that an axis
came back; test_plot_logo_color_dict_still_per_character makes the same
kind of call and checks the colors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…omplement does

test_beam_substitution_max_iter_default checked only the shape and dtype,
which hold when no iteration runs; it now checks that the sequence changed
and that its prediction moved toward y. test_beam_substitution_input_mask
checked only that positions outside the mask were unchanged; it now checks
how many changed inside it. test_greedy_substitution_reverse_complement
never passed reverse_complement=True, and its counts held whenever A and C
were added; it now compares the search with the one over the motifs and
their reverse complements listed explicitly, and with the forward-only
search. The first line of test_screen_raises asserted torch's Conv1d
channel error, and test_greedy_substitution_no_y set an unused y.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The only test of the console script is marked cmd, which pyproject's
addopts and the CI command deselect, so main() never ran in a test. Three
tests call it in process with a temporary destination: --print-path, an
install with --dest, and a second install that fails without --force and
succeeds with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k the toy models

- test_deep_lift_shap_random_state compared 16 examples with 10, so its
  "differs" assertion passed on the shape mismatch alone.
- test_hypothetical_attributions_raises lacked a comma, so its numpy
  multipliers case repeated the not-a-tuple case; the "Shape of all
  tensors must match." branch is now tested.
- The two on-error cleanup tests raised on target=5 before any backward
  hook ran, so they passed with a cleanup that only removed the handles.
  The hook test now raises inside the softmax rule; both check every
  attribute _clear_hooks removes. The BilinearOp rows, which could not
  reach the cleanup, are dropped.
- Three tests asserted the fixture's dtype instead of the attributions'.
- test_deep_lift_shap_rules_input_type ran 45 calls whose inputs reach
  each model as the same values; its output-dtype check moves into
  test_deep_lift_shap_input_type. test_deep_lift_shap_rules_return_references
  compared a NamedTuple field with itself; the check that a references
  tensor comes back unchanged moves into the named-tuple test.
- test_windows_overlap_defaults_stride_to_kernel_size took an unused
  device and repeated a row of the 1d test; its 2d case joins the 2d test.
- test_transformer_block_attention_mask_forward ran only the toy model,
  and test_deep_lift_shap_attention_mask_is_not_vacuous ran eight
  attributions to check that the toy model applies its mask, which a
  forward pass now checks.
- test_deep_lift_shap_subclassed_op is a strict xfail instead of a skip,
  so it reports when subclassed ops are fixed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…their axes

test_pisa_n_shuffles called only deep_lift_shap, and so did the "tiny
threshold warns" lines of test_pisa_convergence and the two convdense
wrapper tests; they call pisa now. In test_pisa_raises the reference
tensors came from a two-sequence fixture while X had sixteen, so the
channel and length lines failed on the batch axis; they are built from
the test's own X and each is matched to its shape. Two lines that
repeated earlier ones are dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…real bin layout

The out-of-range lines of the insert and substitute raise_ends tests
passed an int as the fourth positional argument, alphabet, so their
TypeError came from one_hot_encode. The raises_alphabet motifs and the
one-hot line of the length test had no batch axis, so they failed on the
number of axes and never reached the channel or length check; they get
one, and each is matched to its message. The second line of each
raises_alphabet test passed the alphabet where it binds to start.
test_local_dinucleotide_shuffle_bin_layout_valid checked the test file's
own helper; it now checks the bins the library draws, over 200 seeds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_saturation_mutagenesis_ordering compared at two decimals
attributions whose largest magnitude is about 2e-3, so it passed against
zeros; it compares at four. test_saturation_mutagenesis_packed never
reached the fast path with fast=True, since a job this small skips it, so
its eight cases checked only the decode; it now forces the fast path and
fails on a fallback warning. test_saturation_mutagenesis_no_warning_on_hard_one_hot
made the same call, under the same warnings-as-errors filter, as
test_saturation_mutagenesis_fast_skips_small_jobs and
test_saturation_mutagenesis_fast_false_never_captures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vice

test_predict_raises_shape asserted the RuntimeError raised by the toy
model's permute; predict does not check the shape. test_predict_empty_X,
test_predict_packed_raises and test_predict_raises_args raise before
anything is moved to a device, so their cuda cases repeated the cpu ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er use it

The ablate tests of invalid shapes, arguments, deep_lift_shap keywords and
empty annotations raise before anything is moved, and
test_variant_effect_returns_named_tuple never passes device, so their
cuda cases repeated the cpu ones. test_ablate_deep_lift_shap_raises needs
a device keyword to collide with additional_func_kwargs, so it passes
'cpu'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_marginalize_additional_func_kwargs_none compared a call with
additional_func_kwargs=None, the default, with the same call without it.
test_marginalize_annotations_threads_kwargs compared batch sizes 1 and 64,
which give equal predictions whether or not the keywords reach
marginalize; test_marginalize_annotations_deep_lift_shap checks that they
do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…atch differently

Batch sizes are capped at the number of sequences, so the 64 and 10000
cases of test_dependency_map_batch_size ran the same single batch as the
reference call, and the 20 and 64 cases of test_saliency_batch_size the
same as 10000. dependency_map now runs 1, 5 and 7 on its 24 edits, and
saliency keeps 1, 3 and 10000.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jmschrei
jmschrei force-pushed the test_fix_vacuous_checks branch from 92f67a9 to 4612fe4 Compare October 19, 2026 19:07
@jmschrei
jmschrei merged commit c635142 into main Oct 19, 2026
4 checks passed
@jmschrei
jmschrei deleted the test_fix_vacuous_checks branch October 19, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant