Skip to content

Commit 9b5980b

Browse files
hyperpolymathclaude
hyperpolymath
and
claude
committed
fix(presswerk): eliminate .expect("TODO") anti-pattern across 69 test sites
Applied structural error-handling policies to replace all `.expect("TODO: handle error")` instances with context-appropriate fixes: - Policy A (let-else): document conversion tests, status message assertions - Policy B (?-propagation): audit log tests using Result-returning closures - Policy D (match pivot): parse failures and attribute lookups with explicit let-Some - Policy E (invariant): hardcoded socket addresses, mutex locks, last() on arrays Site breakdown: presswerk-security/audit.rs: 12 sites → closure-wrapped ?-propagation presswerk-print/health.rs: 2 sites → let-Some guards with panic presswerk-print/revival.rs: 2 sites → let-Some guards for parse results presswerk-print/ipp_server.rs: 51 sites → let-Some guards + improved panic msgs presswerk-document/convert.rs: 2 sites → let-Ok guards for auto_convert Test results: 92/92 passed, 0 failed - presswerk-security: 14 passed - presswerk-print: 68 passed - presswerk-document: 10 passed Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent f7413a8 commit 9b5980b

5 files changed

Lines changed: 121 additions & 98 deletions

File tree

‎crates/presswerk-document/src/convert.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,10 @@ mod tests {
201201
let mut supported = HashSet::new();
202202
supported.insert("application/pdf".into());
203203

204-
let (result, doc_type) =
205-
DocumentConverter::auto_convert(bytes, DocumentType::Pdf, &supported).expect("TODO: handle error");
204+
let Ok((result, doc_type)) =
205+
DocumentConverter::auto_convert(bytes, DocumentType::Pdf, &supported) else {
206+
panic!("auto_convert failed for native format");
207+
};
206208
assert_eq!(result, bytes);
207209
assert_eq!(doc_type, DocumentType::Pdf);
208210
}
@@ -212,8 +214,10 @@ mod tests {
212214
let bytes = b"test data";
213215
let supported = HashSet::new();
214216

215-
let (result, doc_type) =
216-
DocumentConverter::auto_convert(bytes, DocumentType::Pdf, &supported).expect("TODO: handle error");
217+
let Ok((result, doc_type)) =
218+
DocumentConverter::auto_convert(bytes, DocumentType::Pdf, &supported) else {
219+
panic!("auto_convert failed for empty supported");
220+
};
217221
assert_eq!(result, bytes);
218222
assert_eq!(doc_type, DocumentType::Pdf);
219223
}

‎crates/presswerk-print/src/health.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,10 @@ mod tests {
250250

251251
tracker.record_success(uri);
252252
assert!(tracker.allow_request(uri));
253-
assert_eq!(
254-
tracker.get_health(uri).expect("TODO: handle error").consecutive_failures,
255-
0
256-
);
253+
let Some(health) = tracker.get_health(uri) else {
254+
panic!("no health status for {uri}");
255+
};
256+
assert_eq!(health.consecutive_failures, 0);
257257
}
258258

259259
#[test]
@@ -265,9 +265,10 @@ mod tests {
265265
tracker.record_failure(uri, "timeout");
266266
}
267267

268-
let msg = tracker.status_message(uri);
269-
assert!(msg.is_some());
270-
assert!(msg.expect("TODO: handle error").contains("having trouble"));
268+
let Some(msg) = tracker.status_message(uri) else {
269+
panic!("expected status message for {uri}");
270+
};
271+
assert!(msg.contains("having trouble"));
271272
}
272273

273274
#[test]

‎crates/presswerk-print/src/ipp_server.rs‎

Lines changed: 54 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,7 +1599,7 @@ mod tests {
15991599
99
16001600
);
16011601
// Last byte is end-of-attributes
1602-
assert_eq!(*bytes.last().expect("TODO: handle error"), TAG_END_OF_ATTRIBUTES);
1602+
assert_eq!(bytes.last().copied().unwrap_or_else(|| panic!("empty bytes")), TAG_END_OF_ATTRIBUTES);
16031603
}
16041604

16051605
#[test]
@@ -1673,7 +1673,7 @@ mod tests {
16731673
<ipp body here>";
16741674
let result = parse_http_envelope(http);
16751675
assert!(result.is_some());
1676-
let req = result.expect("TODO: handle error");
1676+
let Some(req) = result else { panic!("parse_http_envelope failed") }; let req = req;
16771677
assert_eq!(req.content_length, Some(42));
16781678
assert!(req.body_offset > 0);
16791679
assert_eq!(&http[req.body_offset..], b"<ipp body here>");
@@ -1774,11 +1774,11 @@ mod tests {
17741774
fn dispatch_get_printer_attributes_returns_ok() {
17751775
let state = make_shared_state();
17761776
let data = build_test_ipp_request(OP_GET_PRINTER_ATTRIBUTES, 50, &[], &[]);
1777-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1778-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1777+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1778+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
17791779

17801780
let response = dispatch_operation(&req, peer, &state);
1781-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1781+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
17821782

17831783
// Status should be successful-ok.
17841784
assert_eq!(parsed.operation_id, STATUS_OK);
@@ -1803,11 +1803,11 @@ mod tests {
18031803
fn dispatch_validate_job_returns_ok() {
18041804
let state = make_shared_state();
18051805
let data = build_test_ipp_request(OP_VALIDATE_JOB, 12, &[], &[]);
1806-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1807-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1806+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1807+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
18081808

18091809
let response = dispatch_operation(&req, peer, &state);
1810-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1810+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
18111811

18121812
assert_eq!(parsed.operation_id, STATUS_OK);
18131813
assert_eq!(parsed.request_id, 12);
@@ -1822,11 +1822,11 @@ mod tests {
18221822
(VALUE_TAG_KEYWORD, "document-format", b"application/pdf"),
18231823
];
18241824
let data = build_test_ipp_request(OP_PRINT_JOB, 20, &attrs, doc);
1825-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1826-
let peer: SocketAddr = "192.168.1.50:54321".parse().expect("TODO: handle error");
1825+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1826+
let peer: SocketAddr = "192.168.1.50:54321".parse().expect("hardcoded address is invalid");
18271827

18281828
let response = dispatch_operation(&req, peer, &state);
1829-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1829+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
18301830

18311831
// Should succeed.
18321832
assert_eq!(parsed.operation_id, STATUS_OK);
@@ -1843,8 +1843,8 @@ mod tests {
18431843
assert!(ipp_job_id > 0);
18441844

18451845
// Verify the job was inserted into the queue.
1846-
let queue = state.job_queue.lock().expect("TODO: handle error");
1847-
let all_jobs = queue.get_all_jobs().expect("TODO: handle error");
1846+
let queue = state.job_queue.lock().expect("mutex poisoned");
1847+
let all_jobs = queue.get_all_jobs().expect("get_all_jobs failed");
18481848
assert_eq!(all_jobs.len(), 1);
18491849
assert_eq!(all_jobs[0].document_name, "Test Doc");
18501850
}
@@ -1856,32 +1856,33 @@ mod tests {
18561856
// First, submit a job.
18571857
let doc = b"some data";
18581858
let data = build_test_ipp_request(OP_PRINT_JOB, 30, &[], doc);
1859-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1860-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1859+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1860+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
18611861
let response = dispatch_operation(&req, peer, &state);
1862-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1863-
let job_group = parsed
1862+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
1863+
let Some(job_group) = parsed
18641864
.attribute_groups
18651865
.iter()
1866-
.find(|g| g.delimiter == TAG_JOB_ATTRIBUTES)
1867-
.expect("TODO: handle error");
1868-
let ipp_job_id = job_group.get_integer("job-id").expect("TODO: handle error");
1866+
.find(|g| g.delimiter == TAG_JOB_ATTRIBUTES) else {
1867+
panic!("job attributes group missing from response");
1868+
};
1869+
let ipp_job_id = job_group.get_integer("job-id").expect("job-id attribute missing");
18691870

18701871
// Now cancel it.
18711872
let job_id_bytes = ipp_job_id.to_be_bytes();
18721873
let cancel_attrs = vec![(VALUE_TAG_INTEGER, "job-id", &job_id_bytes[..])];
18731874
let cancel_data = build_test_ipp_request(OP_CANCEL_JOB, 31, &cancel_attrs, &[]);
1874-
let cancel_req = parse_ipp_request(&cancel_data).expect("TODO: handle error");
1875+
let cancel_req = parse_ipp_request(&cancel_data).expect("parse_ipp_request failed");
18751876

18761877
let cancel_response = dispatch_operation(&cancel_req, peer, &state);
1877-
let cancel_parsed = parse_ipp_request(&cancel_response).expect("TODO: handle error");
1878+
let cancel_parsed = parse_ipp_request(&cancel_response).expect("parse_ipp_request failed");
18781879

18791880
assert_eq!(cancel_parsed.operation_id, STATUS_OK);
18801881
assert_eq!(cancel_parsed.request_id, 31);
18811882

18821883
// Verify the job status is now Cancelled.
1883-
let queue = state.job_queue.lock().expect("TODO: handle error");
1884-
let all_jobs = queue.get_all_jobs().expect("TODO: handle error");
1884+
let queue = state.job_queue.lock().expect("mutex poisoned");
1885+
let all_jobs = queue.get_all_jobs().expect("get_all_jobs failed");
18851886
assert_eq!(all_jobs.len(), 1);
18861887
assert_eq!(all_jobs[0].status, JobStatus::Cancelled);
18871888
}
@@ -1892,11 +1893,11 @@ mod tests {
18921893
let job_id_bytes = 9999i32.to_be_bytes();
18931894
let attrs = vec![(VALUE_TAG_INTEGER, "job-id", &job_id_bytes[..])];
18941895
let data = build_test_ipp_request(OP_CANCEL_JOB, 40, &attrs, &[]);
1895-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1896-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1896+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1897+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
18971898

18981899
let response = dispatch_operation(&req, peer, &state);
1899-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1900+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
19001901

19011902
assert_eq!(parsed.operation_id, STATUS_CLIENT_ERROR_NOT_FOUND);
19021903
}
@@ -1905,11 +1906,11 @@ mod tests {
19051906
fn dispatch_get_jobs_returns_empty_list() {
19061907
let state = make_shared_state();
19071908
let data = build_test_ipp_request(OP_GET_JOBS, 60, &[], &[]);
1908-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1909-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1909+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1910+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
19101911

19111912
let response = dispatch_operation(&req, peer, &state);
1912-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1913+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
19131914

19141915
assert_eq!(parsed.operation_id, STATUS_OK);
19151916
// Only operation-attributes group, no job groups.
@@ -1919,22 +1920,22 @@ mod tests {
19191920
#[test]
19201921
fn dispatch_get_jobs_after_print() {
19211922
let state = make_shared_state();
1922-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1923+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
19231924

19241925
// Submit two jobs.
19251926
for i in 0..2 {
19261927
let name_bytes = format!("Job {i}");
19271928
let attrs = vec![(VALUE_TAG_NAME, "job-name", name_bytes.as_bytes())];
19281929
let data = build_test_ipp_request(OP_PRINT_JOB, 100 + i as u32, &attrs, b"data");
1929-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1930+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
19301931
dispatch_operation(&req, peer, &state);
19311932
}
19321933

19331934
// Get-Jobs should return both.
19341935
let data = build_test_ipp_request(OP_GET_JOBS, 200, &[], &[]);
1935-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1936+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
19361937
let response = dispatch_operation(&req, peer, &state);
1937-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1938+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
19381939

19391940
assert_eq!(parsed.operation_id, STATUS_OK);
19401941
// 1 operation-attributes group + 2 job-attributes groups = 3
@@ -1951,11 +1952,11 @@ mod tests {
19511952
let state = make_shared_state();
19521953
// Use a non-existent operation ID.
19531954
let data = build_test_ipp_request(0x00FF, 70, &[], &[]);
1954-
let req = parse_ipp_request(&data).expect("TODO: handle error");
1955-
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("TODO: handle error");
1955+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
1956+
let peer: SocketAddr = "127.0.0.1:12345".parse().expect("hardcoded address is invalid");
19561957

19571958
let response = dispatch_operation(&req, peer, &state);
1958-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
1959+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
19591960

19601961
assert_eq!(
19611962
parsed.operation_id,
@@ -1995,7 +1996,7 @@ mod tests {
19951996
let bytes = builder.build();
19961997

19971998
// Parse it back and verify the second value has an empty name.
1998-
let parsed = parse_ipp_request(&bytes).expect("TODO: handle error");
1999+
let parsed = parse_ipp_request(&bytes).expect("parse_ipp_request failed");
19992000
let group = &parsed.attribute_groups[0];
20002001

20012002
// First attribute: "test-attr" with value "first-value"
@@ -2022,11 +2023,11 @@ mod tests {
20222023
(VALUE_TAG_KEYWORD, "document-format", b"application/pdf"),
20232024
];
20242025
let data = build_test_ipp_request(OP_PRINT_JOB, 200, &attrs, doc);
2025-
let req = parse_ipp_request(&data).expect("TODO: handle error");
2026-
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("TODO: handle error");
2026+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
2027+
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("hardcoded address is invalid");
20272028

20282029
let response = dispatch_operation(&req, peer, &state);
2029-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
2030+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
20302031
assert_eq!(parsed.operation_id, STATUS_OK);
20312032

20322033
// Compute the expected hash.
@@ -2051,16 +2052,16 @@ mod tests {
20512052
let state = make_shared_state_with_dir(tmp.path());
20522053

20532054
let doc = b"identical content for dedup test";
2054-
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("TODO: handle error");
2055+
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("hardcoded address is invalid");
20552056

20562057
// Submit the same document data twice (different job names).
20572058
for i in 0..2u32 {
20582059
let name = format!("Dedup Test {i}");
20592060
let attrs = vec![(VALUE_TAG_NAME, "job-name", name.as_bytes())];
20602061
let data = build_test_ipp_request(OP_PRINT_JOB, 300 + i, &attrs, doc);
2061-
let req = parse_ipp_request(&data).expect("TODO: handle error");
2062+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
20622063
let response = dispatch_operation(&req, peer, &state);
2063-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
2064+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
20642065
assert_eq!(parsed.operation_id, STATUS_OK);
20652066
}
20662067

@@ -2086,11 +2087,11 @@ mod tests {
20862087

20872088
// Submit a job with empty document data.
20882089
let data = build_test_ipp_request(OP_PRINT_JOB, 400, &[], &[]);
2089-
let req = parse_ipp_request(&data).expect("TODO: handle error");
2090-
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("TODO: handle error");
2090+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
2091+
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("hardcoded address is invalid");
20912092

20922093
let response = dispatch_operation(&req, peer, &state);
2093-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
2094+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
20942095
assert_eq!(parsed.operation_id, STATUS_OK);
20952096

20962097
// The documents directory should have no files (empty data is hashed
@@ -2120,9 +2121,9 @@ mod tests {
21202121

21212122
// Manually write a document file.
21222123
let documents_dir = tmp.path().join("documents");
2123-
std::fs::create_dir_all(&documents_dir).expect("TODO: handle error");
2124+
std::fs::create_dir_all(&documents_dir).expect("filesystem operation failed");
21242125
let content = b"test document bytes";
2125-
std::fs::write(documents_dir.join("deadbeef.dat"), content).expect("TODO: handle error");
2126+
std::fs::write(documents_dir.join("deadbeef.dat"), content).expect("filesystem operation failed");
21262127

21272128
let retrieved = server
21282129
.retrieve_document("deadbeef")
@@ -2146,16 +2147,16 @@ mod tests {
21462147
let server = IppServer::new(None, Some(tmp.path().to_path_buf()));
21472148

21482149
// Ensure documents dir exists (normally done by start()).
2149-
std::fs::create_dir_all(tmp.path().join("documents")).expect("TODO: handle error");
2150+
std::fs::create_dir_all(tmp.path().join("documents")).expect("filesystem operation failed");
21502151

21512152
let doc = b"roundtrip content verification payload";
21522153
let attrs = vec![(VALUE_TAG_NAME, "job-name", b"Roundtrip Test" as &[u8])];
21532154
let data = build_test_ipp_request(OP_PRINT_JOB, 500, &attrs, doc);
2154-
let req = parse_ipp_request(&data).expect("TODO: handle error");
2155-
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("TODO: handle error");
2155+
let req = parse_ipp_request(&data).expect("parse_ipp_request failed");
2156+
let peer: SocketAddr = "10.0.0.1:9999".parse().expect("hardcoded address is invalid");
21562157

21572158
let response = dispatch_operation(&req, peer, &state);
2158-
let parsed = parse_ipp_request(&response).expect("TODO: handle error");
2159+
let parsed = parse_ipp_request(&response).expect("parse_ipp_request failed");
21592160
assert_eq!(parsed.operation_id, STATUS_OK);
21602161

21612162
// Compute the hash the same way the server does.

‎crates/presswerk-print/src/revival.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,17 @@ mod tests {
139139

140140
#[test]
141141
fn parse_mac_colon_format() {
142-
let mac = parse_mac("AA:BB:CC:DD:EE:FF").expect("TODO: handle error");
142+
let Some(mac) = parse_mac("AA:BB:CC:DD:EE:FF") else {
143+
panic!("failed to parse valid MAC address");
144+
};
143145
assert_eq!(mac, [0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF]);
144146
}
145147

146148
#[test]
147149
fn parse_mac_dash_format() {
148-
let mac = parse_mac("11-22-33-44-55-66").expect("TODO: handle error");
150+
let Some(mac) = parse_mac("11-22-33-44-55-66") else {
151+
panic!("failed to parse valid MAC address");
152+
};
149153
assert_eq!(mac, [0x11, 0x22, 0x33, 0x44, 0x55, 0x66]);
150154
}
151155

0 commit comments

Comments
 (0)