Skip to content

Reorganize the test suite: file layout, shared scaffolding, and core-module coverage #103

Description

@cjfields

Current state

tests/ holds exactly one integration binary:

file tests lines
tests/dada_pseudo.rs 17 1063

Its name describes about 5 of those 17. The rest have accreted there because that
is where the CLI scaffolding lives:

area tests
pseudo-pooling matches_manual_recipe, reestimate_err_is_opt_in_and_recorded, output_feeds_downstream_steps, is_deterministic_across_sample_jobs, streaming_matches_cached
determinism dada_pooled_is_deterministic_across_threads, merge_pairs_is_deterministic_across_threads
merge-pairs merge_pairs_rescue_unmerged_concatenates
dada I/O equivalence dada_multi_input_matches_per_file_runs, dada_from_fastq_matches_dada_from_derep_json
input validation / CLI contract dada_input_output_guards, dada_rejects_non_acgt_with_a_clear_error, dada_extends_error_model_for_out_of_range_quality, cli_errors_use_the_documented_format
other dada_homo_gap_defaults_to_gap_penalty, derep_orders_by_abundance_then_lexical, dada_failed_uniques_matches_map_nulls

Unit tests: 98 across 13 src modules.

Three separate problems

1. Name/content mismatch and file size. Someone looking for the non-ACGT
validation test, the error-format contract, or derep ordering has no reason to
open dada_pseudo.rs. At 1063 lines it is also getting awkward to navigate.

2. Scaffolding is trapped in one binary. fixture, scratch, run,
run_expect_err, param_i64, asv_set, fasta_seqs, shared_err_model are all
private to that file, which is why unrelated tests keep landing in it. Splitting
needs tests/common/mod.rs first.

3. The algorithm core has no unit tests. Worth stating plainly, because the
file layout is cosmetic and this is not. Modules with zero #[test]:

  • dada.rs, cluster.rs, pval.rs, containers.rs — the partition, the bud/
    shuffle loop, the p-value math
  • error.rs, merge_pairs.rs, remove_bimera.rs, filter_trim.rs, misc.rs

These are currently covered only indirectly, through CLI integration tests and
the concordance guardrail. That catches end-to-end regressions but localizes
nothing: a bug in b_shuffle_converge's tie-break surfaces as "3 ASVs differ in
sample 7". The well-tested modules are the ones with pure, easily-called
functions (nwalign.rs 25, remove_primers.rs 19, error_models.rs 13), which
suggests the gap is about testability of the entry points, not neglect.

A constraint worth knowing before splitting

shared_err_model() uses OnceLock, so it runs learn-errors once per test
binary
. Splitting into N binaries multiplies that: ~0.54s per build measured on
the committed fixtures. Cheap now (whole suite: 1.79s of test time, ~27s wall
including compilation), and compilation of N small binaries may cost more than
the learn runs — but it means "one file per subcommand" is the wrong granularity.
Group by area, not by command.

Suggested shape (not prescriptive)

tests/common/mod.rs        scaffolding: fixture, scratch, run, run_expect_err,
                           asv_set, fasta_seqs, shared_err_model
tests/cli_contract.rs      arg guards, input validation, error format
tests/denoise_modes.rs     dada / dada-pooled / dada-pseudo equivalences
tests/determinism.rs       thread- and sample-jobs-invariance across subcommands
tests/pipeline_io.rs       derep ordering, fastq-vs-json, failed-uniques,
                           merge-pairs, downstream-tag acceptance

Scope

Mechanical for (1) and (2): moving tests, no behaviour change, and the suite must
stay green with the same test names so history stays greppable. (3) is a separate,
larger piece of work and should probably be its own issue once the layout settles
— unit-testing run_dada means constructing DadaParams + Raws directly, which
is worth doing but is not a file move.

Low priority: nothing is broken. Worth doing the next time someone adds a test and
has to think about where it goes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestlow priorityLower priority tickets; good to check but not necessary

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions