max / makenotwork

9.8 KB · 246 lines History Blame Raw

1	//! CI guard for test-suite conventions, the same ratchet idea as
2	//! `migration_hygiene.rs` and `frontend_globals.rs` turned on the tests themselves.
3	//!
4	//! Each rule below has a `HIGH_WATER` count that may only go down. Fixing every
5	//! existing violation in one sweep would be a 3,000-line diff nobody could review,
6	//! so instead the seal freezes today's number and fails the build on a new one.
7	//! When you clean up a file, lower the constant to the number the failure reports.
8	//! Never raise one.
9	//!
10	//! Conventions and their rationale live in `CONTRIBUTING.md`; this file only
11	//! counts. It deliberately does no parsing beyond line matching: a rule cheap
12	//! enough to state as "how many lines look like this" is a rule that stays
13	//! honest, and a clever AST check that drifts from the convention is worse than
14	//! no check.
15	//!
16	//! Run with: cargo test --test test_hygiene
17
18	use std::fs;
19	use std::path::{Path, PathBuf};
20
21	/// Loose status assertions across `tests/`: 6 on 2026-08-06, down from 830.
22	///
23	/// `assert!(resp.status.is_success())` passes on a 200 when the handler promised
24	/// 201, and `is_client_error()` passes on the 404 you get when a route silently
25	/// disappears, which is exactly the regression the test existed to catch. The
26	/// exact form, `assert_eq!(resp.status, 403)`, pins the contract.
27	///
28	/// The 824 that went were converted to the code the handler actually returns,
29	/// measured rather than guessed: a temporary `#[track_caller]` probe replaced
30	/// each predicate, the suite ran, and every site whose observations agreed on one
31	/// code was pinned to it. That is also how the compound conditions fell. Half of
32	/// `resp.status.is_redirection() \|\| resp.status.is_success()` is dead the moment
33	/// the handler settles on one, and every checkout POST carrying that line settles
34	/// on 303.
35	///
36	/// It also caught what the loose form was there to hide. Three signup tests and
37	/// the load harness POSTed `/join`, which is GET-only, so a 405 was satisfying
38	/// `is_client_error()` and none of them had ever reached the uniqueness check.
39	/// Those are repointed at `/join/step/account`.
40	///
41	/// The 6 left are the genuinely uncontracted ones, and each carries a comment
42	/// saying why: three branch on the status rather than assert it, one ranges over
43	/// cases that answer with different codes, and two are login paths that render a
44	/// 200 page with the error in the body, where the status is not where the
45	/// contract lives.
46	///
47	/// The count reads all of `tests/`, not just `tests/workflows/`, since 2026-08-06.
48	/// Scoping it to the workflow modules left the two files every workflow test
49	/// depends on unsealed, and both had kept the loose form: `harness/mod.rs` and
50	/// `load/runner.rs` took `is_success() \|\| is_redirection()` on signup, login,
51	/// create-project and create-item, so a route changing shape underneath the whole
52	/// suite would have gone unnoticed in the one place it is least visible. Measured
53	/// the same way as the 824: signup answers 200, login 303, both creates 200, over
54	/// 2,700 observations with no variance.
55	///
56	/// `tests/load/scenarios.rs` is left alone deliberately. Its `is_success()` calls
57	/// are a virtual user deciding whether to continue a cycle, not assertions, and
58	/// they do not match the pattern below anyway (the receiver is a local, not
59	/// `.status`). This file excludes itself for the obvious reason: the strings it
60	/// searches for are in its own source.
61	const LOOSE_STATUS_HIGH_WATER: usize = 6;
62
63	/// `test_`-prefixed test functions across `src/` and `tests/`: 176 on 2026-08-03.
64	///
65	/// The attribute already says it is a test, so the prefix is noise that pushes the
66	/// behavior being asserted further from the start of the name. The rest of the
67	/// suite names the outcome (`lookups_return_none_when_absent`); these are the
68	/// stragglers.
69	const TEST_PREFIX_HIGH_WATER: usize = 176;
70
71	/// Workflow modules over [`MAX_MODULE_LINES`]: 10 on 2026-08-06, was 11 until
72	/// pinning the status assertions collapsed the wrapped `assert!`s in one of them
73	/// back onto a single line.
74	///
75	/// Past this a module stops being findable: nobody locates the existing test for
76	/// a behavior, so they write a second one beside it. The fix is to split by
77	/// domain, which is why `tests/workflows/` has 123 files rather than 12.
78	const OVERSIZED_MODULE_HIGH_WATER: usize = 10;
79
80	/// The point at which a workflow module should have been split.
81	const MAX_MODULE_LINES: usize = 800;
82
83	const WORKFLOWS_DIR: &str = "tests/workflows";
84
85	/// The whole test tree: the harness and the load runner are as much a part of the
86	/// suite's contract as the workflow modules are.
87	const TESTS_DIR: &str = "tests";
88
89	#[test]
90	fn loose_status_assertions_do_not_increase() {
91	let mut per_file: Vec<(String, usize)> = Vec::new();
92	for path in rs_files(Path::new(TESTS_DIR)) {
93	if file_name(&path) == "test_hygiene.rs" {
94	continue;
95	}
96	let text = fs::read_to_string(&path).expect("read test module");
97	let n = text.matches(".status.is_success()").count()
98	+ text.matches(".status.is_client_error()").count();
99	if n > 0 {
100	per_file.push((file_name(&path), n));
101	}
102	}
103	let total: usize = per_file.iter().map(\|(_, n)\| n).sum();
104
105	if total > LOOSE_STATUS_HIGH_WATER {
106	per_file.sort_by_key(\|(_, n)\| std::cmp::Reverse(*n));
107	panic!(
108	"loose status assertions rose from {LOOSE_STATUS_HIGH_WATER} to {total}.\n\
109	Assert the exact code the handler contracts: assert_eq!(resp.status, 403).\n\
110	Heaviest files: {:?}",
111	&per_file[..per_file.len().min(5)],
112	);
113	}
114	assert_eq!(
115	total, LOOSE_STATUS_HIGH_WATER,
116	"loose status assertions fell to {total}. Lower LOOSE_STATUS_HIGH_WATER to {total}.",
117	);
118	}
119
120	#[test]
121	fn prefixed_test_names_do_not_increase() {
122	let mut found: Vec<String> = Vec::new();
123	for dir in ["src", "tests"] {
124	for path in rs_files(Path::new(dir)) {
125	let text = fs::read_to_string(&path).expect("read rust file");
126	for line in text.lines() {
127	let line = line.trim_start();
128	if line.starts_with("fn test_") \|\| line.starts_with("async fn test_") {
129	found.push(format!("{}: {line}", path.display()));
130	}
131	}
132	}
133	}
134	let total = found.len();
135
136	assert!(
137	total <= TEST_PREFIX_HIGH_WATER,
138	"`test_`-prefixed test names rose from {TEST_PREFIX_HIGH_WATER} to {total}.\n\
139	Name the behavior instead: `returns_none_when_absent`, not `test_lookup`.\n\
140	Examples: {:?}",
141	&found[..found.len().min(5)],
142	);
143	assert_eq!(
144	total, TEST_PREFIX_HIGH_WATER,
145	"`test_` prefixes fell to {total}. Lower TEST_PREFIX_HIGH_WATER to {total}.",
146	);
147	}
148
149	#[test]
150	fn oversized_workflow_modules_do_not_increase() {
151	let mut over: Vec<(String, usize)> = rs_files(Path::new(WORKFLOWS_DIR))
152	.into_iter()
153	.map(\|p\| {
154	let lines = fs::read_to_string(&p)
155	.expect("read workflow module")
156	.lines()
157	.count();
158	(file_name(&p), lines)
159	})
160	.filter(\|(_, lines)\| *lines > MAX_MODULE_LINES)
161	.collect();
162	let total = over.len();
163
164	if total > OVERSIZED_MODULE_HIGH_WATER {
165	over.sort_by_key(\|(_, n)\| std::cmp::Reverse(*n));
166	panic!(
167	"workflow modules over {MAX_MODULE_LINES} lines rose from \
168	{OVERSIZED_MODULE_HIGH_WATER} to {total}.\n\
169	Split the module by domain rather than growing it: {over:?}",
170	);
171	}
172	assert_eq!(
173	total, OVERSIZED_MODULE_HIGH_WATER,
174	"oversized modules fell to {total}. Lower OVERSIZED_MODULE_HIGH_WATER to {total}.",
175	);
176	}
177
178	/// Every test module states what surface it covers. A module with no `//!` header
179	/// is one whose reason for existing lives only in whoever wrote it.
180	#[test]
181	fn every_workflow_module_has_a_doc_header() {
182	let missing: Vec<String> = rs_files(Path::new(WORKFLOWS_DIR))
183	.into_iter()
184	.filter(\|p\| file_name(p) != "mod.rs")
185	.filter(\|p\| {
186	let text = fs::read_to_string(p).expect("read workflow module");
187	!text.lines().next().is_some_and(\|l\| l.starts_with("//!"))
188	})
189	.map(\|p\| file_name(&p))
190	.collect();
191
192	assert!(
193	missing.is_empty(),
194	"workflow modules without a `//!` header: {missing:?}\n\
195	Say what surface the module covers and what would break if it were deleted.",
196	);
197	}
198
199	/// `#[ignore]` without a reason is coverage that silently stopped running while
200	/// still costing compile time. The attribute takes a string; use it to say why,
201	/// and how to run the test instead.
202	#[test]
203	fn every_ignored_test_states_a_reason() {
204	let mut bare: Vec<String> = Vec::new();
205	for dir in ["src", "tests"] {
206	for path in rs_files(Path::new(dir)) {
207	let text = fs::read_to_string(&path).expect("read rust file");
208	for (i, line) in text.lines().enumerate() {
209	if line.trim() == "#[ignore]" {
210	bare.push(format!("{}:{}", path.display(), i + 1));
211	}
212	}
213	}
214	}
215	assert!(
216	bare.is_empty(),
217	"`#[ignore]` with no reason string: {bare:?}\n\
218	Write `#[ignore = \"why, and how to run it\"]`.",
219	);
220	}
221
222	/// Every `.rs` file under `dir`, recursively, in a stable order.
223	fn rs_files(dir: &Path) -> Vec<PathBuf> {
224	let mut out = Vec::new();
225	let Ok(entries) = fs::read_dir(dir) else {
226	return out;
227	};
228	for entry in entries.flatten() {
229	let path = entry.path();
230	if path.is_dir() {
231	out.extend(rs_files(&path));
232	} else if path.extension().is_some_and(\|e\| e == "rs") {
233	out.push(path);
234	}
235	}
236	out.sort();
237	out
238	}
239
240	fn file_name(path: &Path) -> String {
241	path.file_name()
242	.expect("workflow path has a file name")
243	.to_string_lossy()
244	.into_owned()
245	}
246