Skip to content

test: make source-scanning tests independent of file layout - #154

Merged
justin13888 merged 1 commit into
masterfrom
test/133-layout-independent-source-scan-tests
Oct 3, 2026
Merged

justin13888 merged 1 commit into
masterfrom
test/133-layout-independent-source-scan-tests

Conversation

@justin13888

@justin13888 justin13888 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The four tests that hold a property of the source text named their files, so splitting a module would move code out of their sight. They now read whole modules off the directory tree at run time, and the guard-site scan matches a call site by its code rather than by its file.

  • src/testing.rs: adds three test-only (#[cfg(test)]) helpers. rust_sources was already here, private to the test module; it moves up a level and now sorts its output. src_root is new. module_sources("a::b") returns src/a/b.rs and every .rs file under src/a/b/, sorted. If the module has no source at all it panics, so a renamed module can't leave a scan reading nothing. Two new tests cover module_sources: one checks a directory module, a module that has both a file and a directory, and a single-file module; the other checks the panic.
  • src/env_guard.rs, Reason census (declared_reasons): drops include_str!("env_guard.rs"). It now reads every file of the env_guard module and finds the declaration by a line whose trimmed text is exactly pub enum Reason {. Exactly one such line must exist. The parsing in reasons_declared_in and its cross-check are unchanged.
  • src/env_guard.rs, every_site_that_reaches_the_guard_is_known:
    • "This module" now means src/env_guard.rs plus everything beneath src/env_guard/, instead of one file path.
    • KNOWN is now a multiset of code lines with no file names. Each entry is claimed by one site. A site moved into another file still passes. A known line repeated anywhere is reported as a new caller.
    • It uses the shared rust_sources walk instead of its own.
  • src/config/merge.rs, no_config_module_reads_the_environment:
    • Scans every file of the config module plus the paths module, instead of a fixed include_str! list of seven files.
    • The two paths edge lines are still allowed only inside paths, each exactly once, now counted across the whole module.
  • src/config/values/local.rs, the_setter_performs_no_io: scans every file of config::values::local instead of include_str!("local.rs").

Deliberate violation run

Six temporary violations were added to the source, and the four tests were run against this branch's tests and then against master's tests. The violations were then removed; none is committed.

Violation master tests this branch
Reason::ProbeVariant with an #[error] and a terminating arm, chained from nothing the_reasons_render_as_sentences fails: declared, but never reached by the chain ...: ["ProbeVariant"] same failure
crate::env_guard::is_relocating("HOME") in src/report.rs every_site_that_reaches_the_guard_is_known fails: the guard has a new caller: [".../src/report.rs:197"] same failure
std::env::var_os("X") in the non-test half of src/config/merge.rs no_config_module_reads_the_environment fails: merge.rs contains env::var`` fails: config/merge.rs contains env::var``
std::fs::read in the non-test half of src/config/values/local.rs the_setter_performs_no_io fails: `std::fs` appeared fails: config/values/local.rs: std::fs appeared
std::env::var_os("X") in src/config/history.rs, which the old list never named passes; the file was not scanned (read from the old list) fails: config/history.rs contains env::var``
use crate::env_guard::is_variable_name; repeated into src/config/history.rs fails; config/history.rs is not a listed file fails: the extra copy is reported as a new caller

The command for both runs was cargo test --lib -- the_reasons_render_as_sentences every_site_that_reaches no_config_module_reads the_setter_performs_no_io. Every violation master catches is still caught here.

Validation

All of these ran at d1deb15 through the repository's hk pre-commit and pre-push hooks:

  • cargo fmt --check passes.
  • scripts/line-check passes: no file newly over 3743 lines.
  • cargo clippy --all-targets -- -D warnings passes.
  • cargo test passes: lib reports 2036 passed, 0 failed, 14 ignored; every other test binary is green.
  • cargo llvm-cov --ignore-filename-regex 'src/main\.rs' --summary-only --fail-under-lines 80 passes at 97.27% lines.
  • convco passes on the commit message.

Risks and rollout

  • Test-only. Every added item is either #[cfg(test)] or inside a mod tests, so the library and binary do not change.
  • A gap in the old scan surfaced, and is now closed. Under the old file-plus-text matching, a known line could appear any number of times in its file. src/shell/activation.rs holds !env_guard::is_relocating(&name) twice: once in the plain name search and once in the search for names split by quotes. Under the multiset the second copy needed its own KNOWN entry, so KNOWN grows from 15 to 16. The test is now stricter, not looser.
  • Which copy gets named when a known line is repeated. The report names whichever copy the sorted walk reaches last. That is not necessarily the newly added one.
  • The config scan is wider. It now also reads config/{mod,env,external,history,origin,path,secrets,shell_options,tool,tree,when}.rs. None of them holds a forbidden token in its non-test half today.
  • Coverage gap: test-only modules in separate files. A module whose tests live in a separate file declared #[cfg(test)] mod tests; would have that file scanned whole, because the non-test half is still found by splitting at the first #[cfg(test)]. No such file exists today. If one is added, the scan fails loudly rather than letting anything through.

Decisions taken

  1. How the scans find their files
    Taken: directory walk at test time (accepted unrebutted)
    Rejected: per-file include_str! lists - every split would have to edit them, and a forgotten edit blinds the test
    Reverses: restore the lists

  2. Where the shared walk lives
    Taken: #[cfg(test)] helpers in src/testing.rs, which already held rust_sources
    Rejected: a copy of the walk in each test, which means four walks to keep in step; a non-cfg(test) helper, which would compile env!("CARGO_MANIFEST_DIR") into the library
    Reverses: inline the walk in each test and delete module_sources and src_root

  3. What a scan reads
    Taken: the whole crate module, meaning src/a/b.rs plus everything under src/a/b/, chosen by module path. The config scan reads all of config and paths, not the seven files the list happened to name.
    Rejected: all of src/, which would pull in modules that do read the environment (the binary edge, detect) and would need a new exception list
    Reverses: narrow [("config", ..), ("paths", ..)] back to named files

  4. How a guard call site is identified
    Taken: by its code as a multiset, so a known line is claimed once per entry
    Rejected: a set of code lines, which would let a known line be copied into any file unnoticed; file plus text, which the issue rules out
    Reverses: key KNOWN on (file, line) again

  5. The violation-run count (review item C1)
    Taken: the count reads six, matching the six rows the table records; no seventh violation, its command, or its outcome exists anywhere in the record, so a seventh row could only be invented. At d1deb15, with no violation applied, the four tests pass: cargo test --lib -- the_reasons_render_as_sentences every_site_that_reaches no_config_module_reads the_setter_performs_no_io reports 4 passed, 0 failed.
    Rejected: adding a seventh row, which would record a run nobody can show happened
    Reverses: run a seventh violation against both test sets and add its row with the count back at seven

Issue

Closes #133

The Reason census, the guard-site scan, and the two config no-I/O and
no-environment scans named their files with include_str! or matched sites
by file name, so splitting a module moved code out of their sight. They
now read every file of the module they hold at run time, and the guard-site
scan matches a site by its code as a multiset rather than by file and text.

Closes #133
@justin13888 justin13888 added the de-slop Repository cleanup: documentation, structure, and test adequacy label Oct 3, 2026
@justin13888
justin13888 merged commit bf12211 into master Oct 3, 2026
5 checks passed
@justin13888
justin13888 deleted the test/133-layout-independent-source-scan-tests branch October 3, 2026 14:22
@github-actions github-actions Bot mentioned this pull request Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

de-slop Repository cleanup: documentation, structure, and test adequacy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make source-scanning tests independent of file layout

1 participant