Skip to content

Commit d611913

Browse files
committed
Add testcase
1 parent 6728325 commit d611913

9 files changed

Lines changed: 302 additions & 50 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/node/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ tracing.workspace = true
2020
wiremock.workspace = true
2121
tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
2222
rustls.workspace = true
23+
tracing-subscriber.workspace = true
2324

2425
[lints]
2526
workspace = true

crates/node/src/patcher.rs

Lines changed: 68 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,7 @@ impl JsonPatcher {
5959

6060
for &(section, section_key) in DEPENDENCY_SECTIONS {
6161
if let Some(serde_json::Value::Object(deps)) = parsed.get(section_key) {
62-
// Find the byte position of this section key in the text
63-
let Some(section_key_pos) = find_json_key_position(text, section_key, 0) else {
64-
continue;
65-
};
66-
67-
// Find the opening { of the section's value object
68-
let search_from = section_key_pos + section_key.len() + 2; // skip past `"key"`
69-
let Some(obj_start) = find_char_skipping_strings(text, '{', search_from) else {
70-
continue;
71-
};
72-
73-
// Find the matching closing }
74-
let Some(obj_end) = find_matching_brace(text, obj_start) else {
62+
let Some((obj_start, obj_end)) = find_section_bounds(text, section_key) else {
7563
continue;
7664
};
7765

@@ -128,17 +116,7 @@ impl JsonPatcher {
128116
continue;
129117
};
130118

131-
// Find the byte position of this section key in the text
132-
let Some(section_key_pos) = find_json_key_position(text, section_key, 0) else {
133-
continue;
134-
};
135-
136-
let search_from = section_key_pos + section_key.len() + 2;
137-
let Some(obj_start) = find_char_skipping_strings(text, '{', search_from) else {
138-
continue;
139-
};
140-
141-
let Some(obj_end) = find_matching_brace(text, obj_start) else {
119+
let Some((obj_start, obj_end)) = find_section_bounds(text, section_key) else {
142120
continue;
143121
};
144122

@@ -198,6 +176,19 @@ impl JsonPatcher {
198176
}
199177
}
200178

179+
/// Find the byte range `(obj_start, obj_end)` of a dependency-section object
180+
/// in the raw JSON text.
181+
///
182+
/// Returns `None` if the section key cannot be located, the opening `{` is
183+
/// missing, or the matching `}` is missing.
184+
fn find_section_bounds(text: &str, section_key: &str) -> Option<(usize, usize)> {
185+
let section_key_pos = find_json_key_position(text, section_key, 0)?;
186+
let search_from = section_key_pos + section_key.len() + 2; // skip past `"key"`
187+
let obj_start = find_char_skipping_strings(text, '{', search_from)?;
188+
let obj_end = find_matching_brace(text, obj_start)?;
189+
Some((obj_start, obj_end))
190+
}
191+
201192
/// Find the byte position of a JSON key string in the text.
202193
///
203194
/// Searches for `"key"` as a JSON key (followed by `:`), starting from `from`.
@@ -988,4 +979,57 @@ mod tests {
988979
let result = find_char_skipping_strings(text, ':', 0);
989980
assert!(result.is_none());
990981
}
982+
983+
#[test]
984+
fn test_find_section_bounds_key_not_found() {
985+
// Section key doesn't exist in text at all
986+
assert!(find_section_bounds("{}", "dependencies").is_none());
987+
}
988+
989+
#[test]
990+
fn test_find_section_bounds_no_brace_after_key() {
991+
// Key exists and is followed by `:`, but no `{` after it (truncated text)
992+
let text = r#"{"dependencies": "#;
993+
assert!(find_section_bounds(text, "dependencies").is_none());
994+
}
995+
996+
#[test]
997+
fn test_find_section_bounds_no_matching_close_brace() {
998+
// Key exists and `{` found, but no matching `}`
999+
let text = r#"{"dependencies": {"#;
1000+
assert!(find_section_bounds(text, "dependencies").is_none());
1001+
}
1002+
1003+
#[test]
1004+
fn test_find_section_bounds_valid() {
1005+
let text = r#"{"dependencies": {"react": "^17.0.0"}}"#;
1006+
let bounds = find_section_bounds(text, "dependencies");
1007+
assert!(bounds.is_some());
1008+
let (start, end) = bounds.unwrap();
1009+
assert_eq!(&text[start..=end], r#"{"react": "^17.0.0"}"#);
1010+
}
1011+
1012+
#[test]
1013+
fn test_scan_version_locations_unicode_escaped_section_key() {
1014+
// serde_json decodes \u0065 → 'e', seeing "dependencies" as the key.
1015+
// But find_section_bounds searches for literal "dependencies" in the
1016+
// raw text and won't find "depend\u0065ncies" — exercises the continue.
1017+
let input = "{\"depend\\u0065ncies\": {\"react\": \"^17.0.0\"}}";
1018+
let locations = JsonPatcher::scan_version_locations(input).unwrap();
1019+
assert!(locations.is_empty());
1020+
}
1021+
1022+
#[test]
1023+
fn test_find_char_skipping_whitespace_empty_from_end() {
1024+
// When `from == text.len()`, the slice is empty → None
1025+
let text = "abc";
1026+
assert_eq!(find_char_skipping_whitespace(text, ':', text.len()), None);
1027+
}
1028+
1029+
#[test]
1030+
fn test_find_next_quote_empty_from_end() {
1031+
// When `from == text.len()`, the slice is empty → None
1032+
let text = "abc";
1033+
assert_eq!(find_next_quote(text, text.len()), None);
1034+
}
9911035
}

crates/node/src/registry.rs

Lines changed: 85 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -192,21 +192,24 @@ impl NpmRegistry {
192192
handles.push(handle);
193193
}
194194

195-
let mut results = Vec::with_capacity(deps.len());
196-
for handle in handles {
197-
match handle.await {
198-
Ok(result) => results.push(result),
199-
Err(e) => {
200-
warn!("task join error: {e}");
201-
}
202-
}
203-
}
204-
195+
let mut results = collect_task_results(handles).await;
205196
results.sort_unstable_by_key(|(idx, _)| *idx);
206197
results
207198
}
208199
}
209200

201+
/// Collect results from spawned tasks, logging any `JoinError`s (e.g. panics).
202+
async fn collect_task_results<T>(handles: Vec<tokio::task::JoinHandle<T>>) -> Vec<T> {
203+
let mut results = Vec::with_capacity(handles.len());
204+
for handle in handles {
205+
match handle.await {
206+
Ok(result) => results.push(result),
207+
Err(e) => warn!("task join error: {e}"),
208+
}
209+
}
210+
results
211+
}
212+
210213
impl Default for NpmRegistry {
211214
fn default() -> Self {
212215
Self::new()
@@ -813,4 +816,76 @@ mod tests {
813816
let result = select_version("*", Some(&latest), &versions, TargetLevel::Patch);
814817
assert_eq!(result, Some("2.0.0".to_owned()));
815818
}
819+
820+
#[tokio::test]
821+
async fn test_resolve_version_non_latest_with_tracing() {
822+
use wiremock::matchers::{method, path};
823+
use wiremock::{Mock, MockServer, ResponseTemplate};
824+
825+
install_crypto_provider();
826+
827+
// Install a trace-level subscriber so trace!() arguments are evaluated.
828+
let subscriber = tracing_subscriber::fmt()
829+
.with_max_level(tracing::Level::TRACE)
830+
.with_test_writer()
831+
.finish();
832+
let _guard = tracing::subscriber::set_default(subscriber);
833+
834+
let mock_server = MockServer::start().await;
835+
Mock::given(method("GET"))
836+
.and(path("/react"))
837+
.respond_with(
838+
ResponseTemplate::new(200)
839+
.set_body_json(npm_response_body("19.0.0", &["17.0.0", "18.0.0", "19.0.0"])),
840+
)
841+
.mount(&mock_server)
842+
.await;
843+
844+
let registry = NpmRegistry::with_base_url(&mock_server.uri());
845+
let dep = DependencySpec {
846+
name: "react".to_owned(),
847+
current_req: "~17.0.0".to_owned(),
848+
section: dependency_check_updates_core::DependencySection::Dependencies,
849+
};
850+
let result = registry
851+
.resolve_version(&dep, TargetLevel::Greatest)
852+
.await
853+
.unwrap();
854+
assert_eq!(result.selected.as_deref(), Some("19.0.0"));
855+
}
856+
857+
#[tokio::test]
858+
async fn test_collect_task_results_join_error() {
859+
// Suppress panic output from the intentionally-panicking task.
860+
let prev_hook = std::panic::take_hook();
861+
std::panic::set_hook(Box::new(|_| {}));
862+
863+
let handles: Vec<tokio::task::JoinHandle<(usize, Result<ResolvedVersion, DcuError>)>> = vec![
864+
tokio::spawn(async {
865+
(
866+
0,
867+
Ok(ResolvedVersion {
868+
latest: Some("1.0.0".into()),
869+
selected: None,
870+
}),
871+
)
872+
}),
873+
tokio::spawn(async { panic!("simulated join error") }),
874+
tokio::spawn(async {
875+
(
876+
2,
877+
Ok(ResolvedVersion {
878+
latest: Some("2.0.0".into()),
879+
selected: None,
880+
}),
881+
)
882+
}),
883+
];
884+
let results = collect_task_results(handles).await;
885+
886+
std::panic::set_hook(prev_hook);
887+
888+
// The panicking task is dropped; only 2 results survive.
889+
assert_eq!(results.len(), 2);
890+
}
816891
}

crates/python/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ thiserror.workspace = true
2020
wiremock.workspace = true
2121
tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
2222
rustls.workspace = true
23+
tracing-subscriber.workspace = true
2324

2425
[lints]
2526
workspace = true

crates/python/src/registry.rs

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,19 +149,24 @@ impl PyPiRegistry {
149149
handles.push(handle);
150150
}
151151

152-
let mut results = Vec::with_capacity(deps.len());
153-
for handle in handles {
154-
match handle.await {
155-
Ok(result) => results.push(result),
156-
Err(e) => warn!("task join error: {e}"),
157-
}
158-
}
159-
152+
let mut results = collect_task_results(handles).await;
160153
results.sort_unstable_by_key(|(idx, _)| *idx);
161154
results
162155
}
163156
}
164157

158+
/// Collect results from spawned tasks, logging any `JoinError`s (e.g. panics).
159+
async fn collect_task_results<T>(handles: Vec<tokio::task::JoinHandle<T>>) -> Vec<T> {
160+
let mut results = Vec::with_capacity(handles.len());
161+
for handle in handles {
162+
match handle.await {
163+
Ok(result) => results.push(result),
164+
Err(e) => warn!("task join error: {e}"),
165+
}
166+
}
167+
results
168+
}
169+
165170
impl Default for PyPiRegistry {
166171
fn default() -> Self {
167172
Self::new()
@@ -339,4 +344,32 @@ mod tests {
339344
install_crypto_provider();
340345
let _registry = PyPiRegistry::default();
341346
}
347+
348+
#[tokio::test]
349+
async fn test_collect_task_results_join_error() {
350+
use dependency_check_updates_core::{DcuError, ResolvedVersion};
351+
352+
// Suppress panic output from the intentionally-panicking task.
353+
let prev_hook = std::panic::take_hook();
354+
std::panic::set_hook(Box::new(|_| {}));
355+
356+
let handles: Vec<tokio::task::JoinHandle<(usize, Result<ResolvedVersion, DcuError>)>> = vec![
357+
tokio::spawn(async {
358+
(
359+
0,
360+
Ok(ResolvedVersion {
361+
latest: Some("1.0.0".into()),
362+
selected: None,
363+
}),
364+
)
365+
}),
366+
tokio::spawn(async { panic!("simulated join error") }),
367+
];
368+
let results = super::collect_task_results(handles).await;
369+
370+
std::panic::set_hook(prev_hook);
371+
372+
// The panicking task is dropped; only 1 result survives.
373+
assert_eq!(results.len(), 1);
374+
}
342375
}

crates/rust/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ thiserror.workspace = true
2121
wiremock.workspace = true
2222
tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
2323
rustls.workspace = true
24+
tracing-subscriber.workspace = true
2425

2526
[lints]
2627
workspace = true

crates/rust/src/parser.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,4 +582,29 @@ features = ["derive"]
582582
assert_eq!(manifest.dependencies[0].name, "serde");
583583
assert_eq!(manifest.dependencies[0].current_req, "1.0");
584584
}
585+
586+
#[test]
587+
fn test_apply_updates_unhandled_value_type() {
588+
// A dependency entry that is an array (not string/inline-table/table)
589+
// hits the catch-all `_ => {}` arm in update_dep_in_table.
590+
let toml = r#"
591+
[dependencies]
592+
weird-dep = [1, 2, 3]
593+
serde = "1.0"
594+
"#;
595+
let mut manifest = CargoTomlManifest::parse(toml).unwrap();
596+
// Array-valued dep is not collected as a dependency
597+
assert_eq!(manifest.dependencies.len(), 1);
598+
599+
// Attempting to update the array-valued dep should succeed silently
600+
let updates = vec![PlannedUpdate {
601+
name: "weird-dep".to_owned(),
602+
section: DependencySection::Dependencies,
603+
from: "1.0".to_owned(),
604+
to: "2.0".to_owned(),
605+
}];
606+
let result = manifest.apply_updates(&updates).unwrap();
607+
// Original value unchanged — the catch-all arm is a no-op
608+
assert!(result.contains("weird-dep = [1, 2, 3]"));
609+
}
585610
}

0 commit comments

Comments
 (0)