Skip to content

Commit a0b0ace

Browse files
nnethercotennethercote
committed
tidy: add unit tests for alphabetical markers.
This requires introducing `tidy_error_ext!` as an alternative to `tidy_error!`. It is passed a closure that is called for an error. This lets tests capture tidy error messages in a buffer instead of printing them to stderr. It also requires pulling part of `check` out into a new function `check_lines`, so that tests can pass in an iterator over some strings instead of a file.
1 parent a79a2c7 commit a0b0ace

File tree

3 files changed

+238
-25
lines changed

3 files changed

+238
-25
lines changed

‎src/tools/tidy/src/alphabetical.rs

+40-19
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ use std::path::Path;
2424

2525
use crate::walk::{filter_dirs, walk};
2626

27+
#[cfg(test)]
28+
mod tests;
29+
2730
fn indentation(line: &str) -> usize {
2831
line.find(|c| c != ' ').unwrap_or(0)
2932
}
@@ -38,6 +41,7 @@ const END_MARKER: &str = "tidy-alphabetical-end";
3841
fn check_section<'a>(
3942
file: impl Display,
4043
lines: impl Iterator<Item = (usize, &'a str)>,
44+
err: &mut dyn FnMut(&str) -> std::io::Result<()>,
4145
bad: &mut bool,
4246
) {
4347
let mut prev_line = String::new();
@@ -50,7 +54,12 @@ fn check_section<'a>(
5054
}
5155

5256
if line.contains(START_MARKER) {
53-
tidy_error!(bad, "{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`", idx + 1);
57+
tidy_error_ext!(
58+
err,
59+
bad,
60+
"{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`",
61+
idx + 1
62+
);
5463
return;
5564
}
5665

@@ -93,32 +102,44 @@ fn check_section<'a>(
93102
let prev_line_trimmed_lowercase = prev_line.trim_start_matches(' ').to_lowercase();
94103

95104
if trimmed_line.to_lowercase() < prev_line_trimmed_lowercase {
96-
tidy_error!(bad, "{file}:{}: line not in alphabetical order", idx + 1);
105+
tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1);
97106
}
98107

99108
prev_line = line;
100109
}
101110

102-
tidy_error!(bad, "{file}: reached end of file expecting `{END_MARKER}`")
111+
tidy_error_ext!(err, bad, "{file}: reached end of file expecting `{END_MARKER}`")
103112
}
104113

105-
pub fn check(path: &Path, bad: &mut bool) {
106-
walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
107-
let file = &entry.path().display();
114+
fn check_lines<'a>(
115+
file: &impl Display,
116+
mut lines: impl Iterator<Item = (usize, &'a str)>,
117+
err: &mut dyn FnMut(&str) -> std::io::Result<()>,
118+
bad: &mut bool,
119+
) {
120+
while let Some((idx, line)) = lines.next() {
121+
if line.contains(END_MARKER) {
122+
tidy_error_ext!(
123+
err,
124+
bad,
125+
"{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`",
126+
idx + 1
127+
)
128+
}
108129

109-
let mut lines = contents.lines().enumerate();
110-
while let Some((idx, line)) = lines.next() {
111-
if line.contains(END_MARKER) {
112-
tidy_error!(
113-
bad,
114-
"{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`",
115-
idx + 1
116-
)
117-
}
118-
119-
if line.contains(START_MARKER) {
120-
check_section(file, &mut lines, bad);
121-
}
130+
if line.contains(START_MARKER) {
131+
check_section(file, &mut lines, err, bad);
122132
}
133+
}
134+
}
135+
136+
pub fn check(path: &Path, bad: &mut bool) {
137+
let skip =
138+
|path: &_, _is_dir| filter_dirs(path) || path.ends_with("tidy/src/alphabetical/tests.rs");
139+
140+
walk(path, skip, &mut |entry, contents| {
141+
let file = &entry.path().display();
142+
let lines = contents.lines().enumerate();
143+
check_lines(file, lines, &mut crate::tidy_error, bad)
123144
});
124145
}

‎src/tools/tidy/src/alphabetical/tests.rs

+188
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
use super::*;
2+
use std::io::Write;
3+
use std::str::from_utf8;
4+
5+
fn test(lines: &str, name: &str, expected_msg: &str, expected_bad: bool) {
6+
let mut actual_msg = Vec::new();
7+
let mut actual_bad = false;
8+
let mut err = |args: &_| {
9+
write!(&mut actual_msg, "{args}")?;
10+
Ok(())
11+
};
12+
check_lines(&name, lines.lines().enumerate(), &mut err, &mut actual_bad);
13+
assert_eq!(expected_msg, from_utf8(&actual_msg).unwrap());
14+
assert_eq!(expected_bad, actual_bad);
15+
}
16+
17+
fn good(lines: &str) {
18+
test(lines, "good", "", false);
19+
}
20+
21+
fn bad(lines: &str, expected_msg: &str) {
22+
test(lines, "bad", expected_msg, true);
23+
}
24+
25+
#[test]
26+
fn test_no_markers() {
27+
let lines = "\
28+
def
29+
abc
30+
xyz
31+
";
32+
good(lines);
33+
}
34+
35+
#[test]
36+
fn test_rust_good() {
37+
let lines = "\
38+
// tidy-alphabetical-start
39+
abc
40+
def
41+
xyz
42+
// tidy-alphabetical-end"; // important: end marker on last line
43+
good(lines);
44+
}
45+
46+
#[test]
47+
fn test_complex_good() {
48+
let lines = "\
49+
zzz
50+
51+
// tidy-alphabetical-start
52+
abc
53+
// Rust comments are ok
54+
def
55+
# TOML comments are ok
56+
xyz
57+
// tidy-alphabetical-end
58+
59+
# tidy-alphabetical-start
60+
foo(abc);
61+
// blank lines are ok
62+
63+
// split line gets joined
64+
foo(
65+
def
66+
);
67+
68+
foo(xyz);
69+
# tidy-alphabetical-end
70+
71+
% tidy-alphabetical-start
72+
abc
73+
ignored_due_to_different_indent
74+
def
75+
% tidy-alphabetical-end
76+
77+
aaa
78+
";
79+
good(lines);
80+
}
81+
82+
#[test]
83+
fn test_rust_bad() {
84+
let lines = "\
85+
// tidy-alphabetical-start
86+
abc
87+
xyz
88+
def
89+
// tidy-alphabetical-end
90+
";
91+
bad(lines, "bad:4: line not in alphabetical order");
92+
}
93+
94+
#[test]
95+
fn test_toml_bad() {
96+
let lines = "\
97+
# tidy-alphabetical-start
98+
abc
99+
xyz
100+
def
101+
# tidy-alphabetical-end
102+
";
103+
bad(lines, "bad:4: line not in alphabetical order");
104+
}
105+
106+
#[test]
107+
fn test_features_bad() {
108+
// Even though lines starting with `#` are treated as comments, lines
109+
// starting with `#!` are an exception.
110+
let lines = "\
111+
tidy-alphabetical-start
112+
#![feature(abc)]
113+
#![feature(xyz)]
114+
#![feature(def)]
115+
tidy-alphabetical-end
116+
";
117+
bad(lines, "bad:4: line not in alphabetical order");
118+
}
119+
120+
#[test]
121+
fn test_indent_bad() {
122+
// All lines are indented the same amount, and so are checked.
123+
let lines = "\
124+
$ tidy-alphabetical-start
125+
abc
126+
xyz
127+
def
128+
$ tidy-alphabetical-end
129+
";
130+
bad(lines, "bad:4: line not in alphabetical order");
131+
}
132+
133+
#[test]
134+
fn test_split_bad() {
135+
let lines = "\
136+
|| tidy-alphabetical-start
137+
foo(abc)
138+
foo(
139+
xyz
140+
)
141+
foo(
142+
def
143+
)
144+
&& tidy-alphabetical-end
145+
";
146+
bad(lines, "bad:7: line not in alphabetical order");
147+
}
148+
149+
#[test]
150+
fn test_double_start() {
151+
let lines = "\
152+
tidy-alphabetical-start
153+
abc
154+
tidy-alphabetical-start
155+
";
156+
bad(lines, "bad:3 found `tidy-alphabetical-start` expecting `tidy-alphabetical-end`");
157+
}
158+
159+
#[test]
160+
fn test_missing_start() {
161+
let lines = "\
162+
abc
163+
tidy-alphabetical-end
164+
abc
165+
";
166+
bad(lines, "bad:2 found `tidy-alphabetical-end` expecting `tidy-alphabetical-start`");
167+
}
168+
169+
#[test]
170+
fn test_missing_end() {
171+
let lines = "\
172+
tidy-alphabetical-start
173+
abc
174+
";
175+
bad(lines, "bad: reached end of file expecting `tidy-alphabetical-end`");
176+
}
177+
178+
#[test]
179+
fn test_double_end() {
180+
let lines = "\
181+
tidy-alphabetical-start
182+
abc
183+
tidy-alphabetical-end
184+
def
185+
tidy-alphabetical-end
186+
";
187+
bad(lines, "bad:5 found `tidy-alphabetical-end` expecting `tidy-alphabetical-start`");
188+
}

‎src/tools/tidy/src/lib.rs

+10-6
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
//! This library contains the tidy lints and exposes it
44
//! to be used by tools.
55
6-
use std::fmt::Display;
7-
86
use termcolor::WriteColor;
97

108
/// A helper macro to `unwrap` a result except also print out details like:
@@ -31,16 +29,22 @@ macro_rules! t {
3129

3230
macro_rules! tidy_error {
3331
($bad:expr, $($fmt:tt)*) => ({
34-
$crate::tidy_error($bad, format_args!($($fmt)*)).expect("failed to output error");
32+
$crate::tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error");
33+
*$bad = true;
3534
});
3635
}
3736

38-
fn tidy_error(bad: &mut bool, args: impl Display) -> std::io::Result<()> {
37+
macro_rules! tidy_error_ext {
38+
($tidy_error:path, $bad:expr, $($fmt:tt)*) => ({
39+
$tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error");
40+
*$bad = true;
41+
});
42+
}
43+
44+
fn tidy_error(args: &str) -> std::io::Result<()> {
3945
use std::io::Write;
4046
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream};
4147

42-
*bad = true;
43-
4448
let mut stderr = StandardStream::stdout(ColorChoice::Auto);
4549
stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?;
4650

0 commit comments

Comments
 (0)