Skip to content

Commit 12d94d3

Browse files
authored
Merge pull request #485 from rustcoreutils/copilot/audit-who-utility-implementation
Audit who utility: fix -t option, add test coverage, enhance output format
2 parents fc4ad9f + f5bee49 commit 12d94d3

File tree

3 files changed

+280
-23
lines changed

3 files changed

+280
-23
lines changed

sys/tests/sys-tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ mod getconf;
22
mod ipcrm;
33
mod ipcs;
44
mod uname;
5+
mod who;

sys/tests/who/mod.rs

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
//
2+
// Copyright (c) 2024 Jeff Garzik
3+
//
4+
// This file is part of the posixutils-rs project covered under
5+
// the MIT License. For the full license text, please see the LICENSE
6+
// file in the root directory of this project.
7+
// SPDX-License-Identifier: MIT
8+
//
9+
10+
use plib::testing::{run_test_with_checker, TestPlan};
11+
use std::process::Output;
12+
13+
fn run_who_test(args: Vec<&str>, expected_exit_code: i32, check_fn: fn(&TestPlan, &Output)) {
14+
let plan = TestPlan {
15+
cmd: "who".to_string(),
16+
args: args.iter().map(|&s| s.to_string()).collect(),
17+
stdin_data: String::new(),
18+
expected_out: String::new(),
19+
expected_err: String::new(),
20+
expected_exit_code,
21+
};
22+
run_test_with_checker(plan, check_fn);
23+
}
24+
25+
// Checker functions
26+
fn check_exit_success(_: &TestPlan, output: &Output) {
27+
assert!(output.status.success(), "Expected successful exit");
28+
}
29+
30+
fn check_has_column_headings(_: &TestPlan, output: &Output) {
31+
let stdout = String::from_utf8_lossy(&output.stdout);
32+
let lines: Vec<&str> = stdout.lines().collect();
33+
34+
// Should have at least a header line
35+
assert!(!lines.is_empty(), "Expected output with headings");
36+
37+
// First line should contain column headers
38+
let first_line = lines[0].to_uppercase();
39+
assert!(
40+
first_line.contains("NAME") || first_line.contains("LINE") || first_line.contains("TIME"),
41+
"Expected header line to contain NAME, LINE, or TIME"
42+
);
43+
}
44+
45+
fn check_summary_format(_: &TestPlan, output: &Output) {
46+
let stdout = String::from_utf8_lossy(&output.stdout);
47+
let lines: Vec<&str> = stdout.lines().collect();
48+
49+
// Summary format should have at least the "# users=" line
50+
assert!(
51+
lines.iter().any(|line| line.contains("# users=")),
52+
"Expected '# users=' line in summary output"
53+
);
54+
}
55+
56+
fn check_output_has_terminal_state(_: &TestPlan, output: &Output) {
57+
let stdout = String::from_utf8_lossy(&output.stdout);
58+
59+
// With -T, output should include terminal state characters (+, -, ?)
60+
// Skip if no output (no logged in users)
61+
if !stdout.trim().is_empty() {
62+
let lines: Vec<&str> = stdout.lines().filter(|l| !l.trim().is_empty()).collect();
63+
if !lines.is_empty() {
64+
// At least one line should potentially have terminal state indicators
65+
// The format should be: NAME STATE LINE TIME
66+
// We can't guarantee specific output, but we can check structure
67+
for line in lines {
68+
// Each line should have multiple fields
69+
let fields: Vec<&str> = line.split_whitespace().collect();
70+
assert!(fields.len() >= 2, "Expected multiple fields in output");
71+
}
72+
}
73+
}
74+
}
75+
76+
#[test]
77+
fn who_no_args() {
78+
// Default behavior: show logged in users
79+
run_who_test(vec![], 0, check_exit_success);
80+
}
81+
82+
#[test]
83+
fn who_short_format() {
84+
// -s is the default, explicit test
85+
run_who_test(vec!["-s"], 0, check_exit_success);
86+
}
87+
88+
#[test]
89+
fn who_heading() {
90+
// -H should print column headings
91+
run_who_test(vec!["-H"], 0, check_has_column_headings);
92+
}
93+
94+
#[test]
95+
fn who_summary() {
96+
// -q should show summary format
97+
run_who_test(vec!["-q"], 0, check_summary_format);
98+
}
99+
100+
#[test]
101+
fn who_boot() {
102+
// -b should show boot time
103+
run_who_test(vec!["-b"], 0, check_exit_success);
104+
}
105+
106+
#[test]
107+
fn who_dead() {
108+
// -d should show dead processes
109+
run_who_test(vec!["-d"], 0, check_exit_success);
110+
}
111+
112+
#[test]
113+
fn who_login() {
114+
// -l should show login processes
115+
run_who_test(vec!["-l"], 0, check_exit_success);
116+
}
117+
118+
#[test]
119+
fn who_process() {
120+
// -p should show active processes spawned by init
121+
run_who_test(vec!["-p"], 0, check_exit_success);
122+
}
123+
124+
#[test]
125+
fn who_runlevel() {
126+
// -r should show current runlevel
127+
run_who_test(vec!["-r"], 0, check_exit_success);
128+
}
129+
130+
#[test]
131+
fn who_time() {
132+
// -t should show last system clock change
133+
run_who_test(vec!["-t"], 0, check_exit_success);
134+
}
135+
136+
#[test]
137+
fn who_terminals() {
138+
// -T should show terminal state
139+
run_who_test(vec!["-T"], 0, check_output_has_terminal_state);
140+
}
141+
142+
#[test]
143+
fn who_users() {
144+
// -u should show idle time for users
145+
run_who_test(vec!["-u"], 0, check_exit_success);
146+
}
147+
148+
#[test]
149+
fn who_all() {
150+
// -a should enable all options
151+
run_who_test(vec!["-a"], 0, check_exit_success);
152+
}
153+
154+
#[test]
155+
fn who_combined_options() {
156+
// Test combining options
157+
run_who_test(vec!["-H", "-b"], 0, check_has_column_headings);
158+
}
159+
160+
#[test]
161+
fn who_current_terminal() {
162+
// -m should show only current terminal
163+
run_who_test(vec!["-m"], 0, check_exit_success);
164+
}
165+
166+
#[test]
167+
fn who_userproc() {
168+
// --userproc should show normal user processes
169+
run_who_test(vec!["--userproc"], 0, check_exit_success);
170+
}

sys/who.rs

Lines changed: 109 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -140,16 +140,50 @@ fn get_idle_time(line: &str) -> String {
140140
}
141141
}
142142

143+
// Clean up the id field - take only first 4 chars or until first non-alphanumeric
144+
fn clean_id_field(id: &str) -> String {
145+
id.chars()
146+
.take_while(|c| c.is_alphanumeric() || *c == '~' || *c == '/')
147+
.take(4)
148+
.collect::<String>()
149+
}
150+
151+
// Get the comment field to display for an entry
152+
fn get_comment_field(entry: &Utmpx) -> String {
153+
let clean_id = clean_id_field(&entry.id);
154+
if !clean_id.is_empty() && entry.typ != platform::USER_PROCESS {
155+
format!("id={}", clean_id)
156+
} else if !entry.host.is_empty() && entry.typ == platform::USER_PROCESS {
157+
format!("({})", entry.host)
158+
} else {
159+
String::new()
160+
}
161+
}
162+
143163
fn print_fmt_short(args: &Args, entry: &Utmpx, line: &str) {
144164
if args.idle_time {
145-
println!(
146-
"{:<16} {:<12} {} {} {:>5}",
147-
entry.user,
148-
line,
149-
fmt_timestamp(entry.timestamp),
150-
get_idle_time(line),
151-
entry.pid
152-
);
165+
let comment = get_comment_field(entry);
166+
167+
if comment.is_empty() {
168+
println!(
169+
"{:<16} {:<12} {} {} {:>5}",
170+
entry.user,
171+
line,
172+
fmt_timestamp(entry.timestamp),
173+
get_idle_time(line),
174+
entry.pid
175+
);
176+
} else {
177+
println!(
178+
"{:<16} {:<12} {} {} {:>5} {}",
179+
entry.user,
180+
line,
181+
fmt_timestamp(entry.timestamp),
182+
get_idle_time(line),
183+
entry.pid,
184+
comment
185+
);
186+
}
153187
} else {
154188
println!(
155189
"{:<16} {:<12} {}",
@@ -163,15 +197,30 @@ fn print_fmt_short(args: &Args, entry: &Utmpx, line: &str) {
163197
fn print_fmt_term(args: &Args, entry: &Utmpx, line: &str) {
164198
let term_state = get_terminal_state(line);
165199
if args.idle_time {
166-
println!(
167-
"{:<16} {} {:<12} {} {} {:>5}",
168-
entry.user,
169-
term_state,
170-
line,
171-
fmt_timestamp(entry.timestamp),
172-
get_idle_time(line),
173-
entry.pid
174-
);
200+
let comment = get_comment_field(entry);
201+
202+
if comment.is_empty() {
203+
println!(
204+
"{:<16} {} {:<12} {} {} {:>5}",
205+
entry.user,
206+
term_state,
207+
line,
208+
fmt_timestamp(entry.timestamp),
209+
get_idle_time(line),
210+
entry.pid
211+
);
212+
} else {
213+
println!(
214+
"{:<16} {} {:<12} {} {} {:>5} {}",
215+
entry.user,
216+
term_state,
217+
line,
218+
fmt_timestamp(entry.timestamp),
219+
get_idle_time(line),
220+
entry.pid,
221+
comment
222+
);
223+
}
175224
} else {
176225
println!(
177226
"{:<16} {} {:<12} {}",
@@ -216,6 +265,8 @@ fn print_entry(args: &Args, entry: &Utmpx) {
216265
|| (args.login && entry.typ == platform::LOGIN_PROCESS)
217266
|| (args.runlevel && entry.typ == platform::RUN_LVL)
218267
|| (args.process && entry.typ == platform::INIT_PROCESS)
268+
|| (args.last_change
269+
&& (entry.typ == platform::OLD_TIME || entry.typ == platform::NEW_TIME))
219270
{
220271
selected = true;
221272
}
@@ -238,12 +289,47 @@ fn print_entry(args: &Args, entry: &Utmpx) {
238289

239290
fn show_utmpx_entries(args: &Args) {
240291
if args.heading {
241-
println!(
242-
"{:<16} {:<12} {}",
243-
gettext("NAME"),
244-
gettext("LINE"),
245-
gettext("TIME")
246-
);
292+
if args.idle_time && args.terminals {
293+
// -H -u -T format
294+
println!(
295+
"{:<16} {} {:<12} {:<16} {:>5} {:>5} {}",
296+
gettext("NAME"),
297+
"",
298+
gettext("LINE"),
299+
gettext("TIME"),
300+
gettext("IDLE"),
301+
gettext("PID"),
302+
gettext("COMMENT")
303+
);
304+
} else if args.idle_time {
305+
// -H -u format
306+
println!(
307+
"{:<16} {:<12} {:<16} {:>5} {:>5} {}",
308+
gettext("NAME"),
309+
gettext("LINE"),
310+
gettext("TIME"),
311+
gettext("IDLE"),
312+
gettext("PID"),
313+
gettext("COMMENT")
314+
);
315+
} else if args.terminals {
316+
// -H -T format
317+
println!(
318+
"{:<16} {} {:<12} {}",
319+
gettext("NAME"),
320+
"",
321+
gettext("LINE"),
322+
gettext("TIME")
323+
);
324+
} else {
325+
// -H format
326+
println!(
327+
"{:<16} {:<12} {}",
328+
gettext("NAME"),
329+
gettext("LINE"),
330+
gettext("TIME")
331+
);
332+
}
247333
}
248334

249335
let entries = utmpx::load();

0 commit comments

Comments
 (0)