max / makenotwork

10.2 KB · 251 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`). `tests/load/metrics.rs` is the same case and takes the same shape:
60	/// its report builder buckets observed statuses into errors and rejections, which
61	/// counts what a run saw rather than asserting what a handler promised. That is
62	/// the line to hold when a new site appears in the load harness. Read a status to
63	/// classify or to branch, bind it to a local and say why; assert one, and pin the
64	/// code. This file excludes itself for the obvious reason: the strings it searches
65	/// for are in its own source.
66	const LOOSE_STATUS_HIGH_WATER: usize = 6;
67
68	/// `test_`-prefixed test functions across `src/` and `tests/`: 176 on 2026-08-03.
69	///
70	/// The attribute already says it is a test, so the prefix is noise that pushes the
71	/// behavior being asserted further from the start of the name. The rest of the
72	/// suite names the outcome (`lookups_return_none_when_absent`); these are the
73	/// stragglers.
74	const TEST_PREFIX_HIGH_WATER: usize = 176;
75
76	/// Workflow modules over [`MAX_MODULE_LINES`]: 10 on 2026-08-06, was 11 until
77	/// pinning the status assertions collapsed the wrapped `assert!`s in one of them
78	/// back onto a single line.
79	///
80	/// Past this a module stops being findable: nobody locates the existing test for
81	/// a behavior, so they write a second one beside it. The fix is to split by
82	/// domain, which is why `tests/workflows/` has 123 files rather than 12.
83	const OVERSIZED_MODULE_HIGH_WATER: usize = 10;
84
85	/// The point at which a workflow module should have been split.
86	const MAX_MODULE_LINES: usize = 800;
87
88	const WORKFLOWS_DIR: &str = "tests/workflows";
89
90	/// The whole test tree: the harness and the load runner are as much a part of the
91	/// suite's contract as the workflow modules are.
92	const TESTS_DIR: &str = "tests";
93
94	#[test]
95	fn loose_status_assertions_do_not_increase() {
96	let mut per_file: Vec<(String, usize)> = Vec::new();
97	for path in rs_files(Path::new(TESTS_DIR)) {
98	if file_name(&path) == "test_hygiene.rs" {
99	continue;
100	}
101	let text = fs::read_to_string(&path).expect("read test module");
102	let n = text.matches(".status.is_success()").count()
103	+ text.matches(".status.is_client_error()").count();
104	if n > 0 {
105	per_file.push((file_name(&path), n));
106	}
107	}
108	let total: usize = per_file.iter().map(\|(_, n)\| n).sum();
109
110	if total > LOOSE_STATUS_HIGH_WATER {
111	per_file.sort_by_key(\|(_, n)\| std::cmp::Reverse(*n));
112	panic!(
113	"loose status assertions rose from {LOOSE_STATUS_HIGH_WATER} to {total}.\n\
114	Assert the exact code the handler contracts: assert_eq!(resp.status, 403).\n\
115	Heaviest files: {:?}",
116	&per_file[..per_file.len().min(5)],
117	);
118	}
119	assert_eq!(
120	total, LOOSE_STATUS_HIGH_WATER,
121	"loose status assertions fell to {total}. Lower LOOSE_STATUS_HIGH_WATER to {total}.",
122	);
123	}
124
125	#[test]
126	fn prefixed_test_names_do_not_increase() {
127	let mut found: Vec<String> = Vec::new();
128	for dir in ["src", "tests"] {
129	for path in rs_files(Path::new(dir)) {
130	let text = fs::read_to_string(&path).expect("read rust file");
131	for line in text.lines() {
132	let line = line.trim_start();
133	if line.starts_with("fn test_") \|\| line.starts_with("async fn test_") {
134	found.push(format!("{}: {line}", path.display()));
135	}
136	}
137	}
138	}
139	let total = found.len();
140
141	assert!(
142	total <= TEST_PREFIX_HIGH_WATER,
143	"`test_`-prefixed test names rose from {TEST_PREFIX_HIGH_WATER} to {total}.\n\
144	Name the behavior instead: `returns_none_when_absent`, not `test_lookup`.\n\
145	Examples: {:?}",
146	&found[..found.len().min(5)],
147	);
148	assert_eq!(
149	total, TEST_PREFIX_HIGH_WATER,
150	"`test_` prefixes fell to {total}. Lower TEST_PREFIX_HIGH_WATER to {total}.",
151	);
152	}
153
154	#[test]
155	fn oversized_workflow_modules_do_not_increase() {
156	let mut over: Vec<(String, usize)> = rs_files(Path::new(WORKFLOWS_DIR))
157	.into_iter()
158	.map(\|p\| {
159	let lines = fs::read_to_string(&p)
160	.expect("read workflow module")
161	.lines()
162	.count();
163	(file_name(&p), lines)
164	})
165	.filter(\|(_, lines)\| *lines > MAX_MODULE_LINES)
166	.collect();
167	let total = over.len();
168
169	if total > OVERSIZED_MODULE_HIGH_WATER {
170	over.sort_by_key(\|(_, n)\| std::cmp::Reverse(*n));
171	panic!(
172	"workflow modules over {MAX_MODULE_LINES} lines rose from \
173	{OVERSIZED_MODULE_HIGH_WATER} to {total}.\n\
174	Split the module by domain rather than growing it: {over:?}",
175	);
176	}
177	assert_eq!(
178	total, OVERSIZED_MODULE_HIGH_WATER,
179	"oversized modules fell to {total}. Lower OVERSIZED_MODULE_HIGH_WATER to {total}.",
180	);
181	}
182
183	/// Every test module states what surface it covers. A module with no `//!` header
184	/// is one whose reason for existing lives only in whoever wrote it.
185	#[test]
186	fn every_workflow_module_has_a_doc_header() {
187	let missing: Vec<String> = rs_files(Path::new(WORKFLOWS_DIR))
188	.into_iter()
189	.filter(\|p\| file_name(p) != "mod.rs")
190	.filter(\|p\| {
191	let text = fs::read_to_string(p).expect("read workflow module");
192	!text.lines().next().is_some_and(\|l\| l.starts_with("//!"))
193	})
194	.map(\|p\| file_name(&p))
195	.collect();
196
197	assert!(
198	missing.is_empty(),
199	"workflow modules without a `//!` header: {missing:?}\n\
200	Say what surface the module covers and what would break if it were deleted.",
201	);
202	}
203
204	/// `#[ignore]` without a reason is coverage that silently stopped running while
205	/// still costing compile time. The attribute takes a string; use it to say why,
206	/// and how to run the test instead.
207	#[test]
208	fn every_ignored_test_states_a_reason() {
209	let mut bare: Vec<String> = Vec::new();
210	for dir in ["src", "tests"] {
211	for path in rs_files(Path::new(dir)) {
212	let text = fs::read_to_string(&path).expect("read rust file");
213	for (i, line) in text.lines().enumerate() {
214	if line.trim() == "#[ignore]" {
215	bare.push(format!("{}:{}", path.display(), i + 1));
216	}
217	}
218	}
219	}
220	assert!(
221	bare.is_empty(),
222	"`#[ignore]` with no reason string: {bare:?}\n\
223	Write `#[ignore = \"why, and how to run it\"]`.",
224	);
225	}
226
227	/// Every `.rs` file under `dir`, recursively, in a stable order.
228	fn rs_files(dir: &Path) -> Vec<PathBuf> {
229	let mut out = Vec::new();
230	let Ok(entries) = fs::read_dir(dir) else {
231	return out;
232	};
233	for entry in entries.flatten() {
234	let path = entry.path();
235	if path.is_dir() {
236	out.extend(rs_files(&path));
237	} else if path.extension().is_some_and(\|e\| e == "rs") {
238	out.push(path);
239	}
240	}
241	out.sort();
242	out
243	}
244
245	fn file_name(path: &Path) -> String {
246	path.file_name()
247	.expect("workflow path has a file name")
248	.to_string_lossy()
249	.into_owned()
250	}
251