max / makenotwork

Credit the contract tests the coverage seal told you to write The seal counts a money or user-data file as untested unless it holds a test attribute or has a sibling tests.rs. Its own failure message says a database-bound module wants a contract test under tests/ instead of a unit test, and then the count refused to see the file you wrote. db/synckit/invitations.rs is the case that surfaced it: 215 lines of token handling, eleven contract tests in tests/workflows/db_synckit_invitations.rs, and the seal reported it untested and failed Sando run 45. So credit a third form of coverage: a file under tests/ whose doc header reads "contract tests for `some::module`", a convention twenty-odd files already follow. That reveals eight files as covered all along, so the high water drops 41 to 34. The seal got more accurate, not looser, and the 34 that remain are real.

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

Author: Max Johnson <me@maxj.phd> · 2026-08-07 21:11 UTC

Signed with PGP, not checked

Commit: 3b143afa03966d8f8146633327d1366fa8f00f18

Parent: 0a2b00a

1 file changed, +79 insertions, -9 deletions

M server/tests/untested_money_paths.rs +79 -9

			@@ -5,11 +5,13 @@
5	5		//! tests. It counts files in those areas that contain no test of any kind and
6	6		//! fails when the number goes up. It cannot make anyone write a good test; it
7	7		//! can stop a new payment handler or sync table from landing with none at all,
8		-	//! which is how the current 41 accumulated, one reasonable-looking file at a
	8	+	//! which is how the current 34 accumulated, one reasonable-looking file at a
9	9		//! time.
10	10		//!
11		-	//! Deliberately dumb. A file either contains a test attribute or it does not.
12		-	//! No coverage instrumentation, no AST walk: a rule cheap enough to state as
	11	+	//! Deliberately dumb. A file counts as covered if it holds a test attribute, if
	12	+	//! its directory has a sibling `tests.rs`, or if a file under `tests/` names it
	13	+	//! as the subject of a contract test. No coverage instrumentation, no AST walk:
	14	+	//! a rule cheap enough to state as
13	15		//! "how many files look like this" is a rule that stays honest, and mutation
14	16		//! testing (`.cargo/mutants.toml`) is where the harder question of whether the
15	17		//! tests are any good gets asked.
			@@ -18,16 +20,22 @@
18	20		//!
19	21		//! Run with: cargo test --test untested_money_paths
20	22
	23	+	use std::collections::HashSet;
21	24		use std::fs;
22	25		use std::path::{Path, PathBuf};
23	26
24		-	/// Money and user-data files with no test at all: 41 on 2026-08-04, down from
25		-	/// 50 the same day.
	27	+	/// Money and user-data files with no test at all: 34 on 2026-08-07, down from
	28	+	/// 41, which was down from 50 on 2026-08-04.
	29	+	///
	30	+	/// The drop from 41 is not seven new tests. It is `declared_contract_subjects`
	31	+	/// below finally crediting the eight files whose tests live in a
	32	+	/// `tests/workflows/db_*.rs` contract file, which the count had been reporting
	33	+	/// as untested all along. The seal got more accurate, not looser.
26	34		///
27	35		/// Lower it when you cover one. Never raise it: a new untested file in these
28	36		/// areas is the thing this seal exists to refuse. If you genuinely need to add
29	37		/// one, the honest move is to write the test, not to bump the constant.
30		-	const UNTESTED_HIGH_WATER: usize = 41;
	38	+	const UNTESTED_HIGH_WATER: usize = 34;
31	39
32	40		/// Anything that moves money or decides what someone is entitled to.
33	41		const MONEY: &[&str] = &[
			@@ -60,6 +68,7 @@
60	68		fn untested_money_and_data_files_do_not_increase() {
61	69		let mut money = Vec::new();
62	70		let mut data = Vec::new();
	71	+	let subjects = declared_contract_subjects();
63	72
64	73		for path in rs_files(Path::new("src")) {
65	74		let rel = path.to_string_lossy().replace('\\', "/");
			@@ -68,7 +77,8 @@
68	77		if !in_money && !in_data {
69	78		continue;
70	79		}
71		-	if has_test(&path) \|\| sibling_tests_file(&path) {
	80	+	if has_test(&path) \|\| sibling_tests_file(&path) \|\| subjects.contains(&module_path_of(&rel))
	81	+	{
72	82		continue;
73	83		}
74	84		if in_money { &mut money } else { &mut data }.push(rel);
			@@ -83,8 +93,9 @@
83	93		"untested money/data files rose from {UNTESTED_HIGH_WATER} to {total} \
84	94		(money {}, data {}).\n\
85	95		A new file on these paths needs a test before it lands. If the logic is \
86		-	async and database-bound, that is a contract test in `tests/db_*_layer.rs`, \
87		-	not a unit test.\n\
	96	+	async and database-bound, that is a contract test in \
	97	+	`tests/workflows/db_*.rs` whose header reads \"contract tests for \
	98	+	`your::module`\", not a unit test.\n\
88	99		Money: {money:#?}\nUser data: {data:#?}",
89	100		money.len(),
90	101		data.len(),
			@@ -119,6 +130,65 @@
119	130		.is_some_and(\|dir\| dir.join("tests.rs").exists())
120	131		}
121	132
	133	+	/// Every module named as the subject of a contract-test file under `tests/`.
	134	+	///
	135	+	/// The convention is a doc header reading "contract tests for `db::foo::bar`",
	136	+	/// which twenty-odd files in `tests/workflows/` already follow. Crediting it
	137	+	/// closes a hole that made this seal contradict its own advice: the failure
	138	+	/// message tells you a database-bound module wants a contract test in `tests/`
	139	+	/// rather than a unit test, and then the count refused to see the file you
	140	+	/// wrote. `db/synckit/invitations.rs` landed on 2026-08-07 with eleven such
	141	+	/// tests and was still reported untested.
	142	+	///
	143	+	/// Deliberately as dumb as the rest of the seal: a declared subject, not an
	144	+	/// inferred one. A test file that does not say what it covers is not credited,
	145	+	/// which keeps the rule cheap to state and hard to satisfy by accident.
	146	+	fn declared_contract_subjects() -> HashSet<String> {
	147	+	let mut subjects = HashSet::new();
	148	+	for path in rs_files(Path::new("tests")) {
	149	+	let Ok(text) = fs::read_to_string(&path) else {
	150	+	continue;
	151	+	};
	152	+	let header: String = text
	153	+	.lines()
	154	+	.take_while(\|l\| l.starts_with("//!") \|\| l.trim().is_empty())
	155	+	.collect::<Vec<_>>()
	156	+	.join(" ");
	157	+	if !header.contains("contract tests for") {
	158	+	continue;
	159	+	}
	160	+	// Every backticked path in the header, so a file covering several
	161	+	// modules ("`db::tips`, `db::license_keys`, `db::pending_refunds`")
	162	+	// credits all of them.
	163	+	//
	164	+	// Underscores are part of a module name, not a separator: without them
	165	+	// `db::pending_refunds` reads as prose and goes uncredited, which is
	166	+	// how the first cut of this seal landed on 35 instead of 34.
	167	+	for chunk in header.split('`').skip(1).step_by(2) {
	168	+	let path_shaped = chunk.contains("::")
	169	+	&& !chunk.ends_with(':')
	170	+	&& chunk
	171	+	.chars()
	172	+	.all(\|c\| c.is_ascii_lowercase() \|\| c.is_ascii_digit() \|\| c == '_' \|\| c == ':');
	173	+	if path_shaped {
	174	+	subjects.insert(chunk.to_string());
	175	+	}
	176	+	}
	177	+	}
	178	+	subjects
	179	+	}
	180	+
	181	+	/// The module path a source file defines, as a contract-test header would spell
	182	+	/// it: `src/db/synckit/invitations.rs` -> `db::synckit::invitations`, and a
	183	+	/// `mod.rs` names its directory rather than itself.
	184	+	fn module_path_of(rel: &str) -> String {
	185	+	rel.trim_start_matches("src/")
	186	+	.trim_end_matches(".rs")
	187	+	.replace('/', "::")
	188	+	.trim_end_matches("::mod")
	189	+	.to_string()
	190	+	}
	191	+
122	192		fn rs_files(dir: &Path) -> Vec<PathBuf> {
123	193		let mut out = Vec::new();
124	194		let mut stack = vec![dir.to_path_buf()];