Skip to content

Commit 356f4af

Browse files
committed
address PR feedback: filter empty env vars per-candidate, add JSON wire-format test, rename tests
- build.rs: push empty-string filter into each candidate of build_id/commit_sha chains so an explicitly-set-but-empty primary var (e.g. PET_COMMIT_SHA from an unset GitHub Actions context) doesn't short-circuit fallbacks. - jsonrpc.rs: add test_info_response_serializes_camel_case_and_omits_none covering camelCase rename + skip_serializing_if without requiring CI env vars at compile time. - rename existing tests to ..._optional_build_metadata for accurate scope.
1 parent b8487d3 commit 356f4af

3 files changed

Lines changed: 44 additions & 5 deletions

File tree

crates/pet/build.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,34 @@ fn main() {
88
println!("cargo:rerun-if-env-changed=BUILD_SOURCEVERSION");
99
println!("cargo:rerun-if-env-changed=GITHUB_SHA");
1010

11+
// Filter empties per-candidate so an explicitly-set-but-empty primary var
12+
// doesn't short-circuit the fallback chain (e.g., `PET_BUILD_ID=""`).
1113
if let Some(build_id) = std::env::var("PET_BUILD_ID")
1214
.ok()
13-
.or_else(|| std::env::var("BUILD_BUILDID").ok())
1415
.filter(|value| !value.is_empty())
16+
.or_else(|| {
17+
std::env::var("BUILD_BUILDID")
18+
.ok()
19+
.filter(|value| !value.is_empty())
20+
})
1521
{
1622
println!("cargo:rustc-env=PET_BUILD_ID={build_id}");
1723
}
1824

1925
// BUILD_SOURCEVERSION is set by Azure Pipelines; GITHUB_SHA by GitHub Actions.
2026
if let Some(commit_sha) = std::env::var("PET_COMMIT_SHA")
2127
.ok()
22-
.or_else(|| std::env::var("BUILD_SOURCEVERSION").ok())
23-
.or_else(|| std::env::var("GITHUB_SHA").ok())
2428
.filter(|value| !value.is_empty())
29+
.or_else(|| {
30+
std::env::var("BUILD_SOURCEVERSION")
31+
.ok()
32+
.filter(|value| !value.is_empty())
33+
})
34+
.or_else(|| {
35+
std::env::var("GITHUB_SHA")
36+
.ok()
37+
.filter(|value| !value.is_empty())
38+
})
2539
{
2640
println!("cargo:rustc-env=PET_COMMIT_SHA={commit_sha}");
2741
}

crates/pet/src/jsonrpc.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1542,7 +1542,7 @@ mod tests {
15421542
}
15431543

15441544
#[test]
1545-
fn test_info_response_uses_package_version_and_optional_build_id() {
1545+
fn test_info_response_uses_package_version_and_optional_build_metadata() {
15461546
let info = InfoResponse::current();
15471547

15481548
assert_eq!(info.pet_version, env!("CARGO_PKG_VERSION"));
@@ -1558,6 +1558,31 @@ mod tests {
15581558
.is_none_or(|commit_sha| !commit_sha.is_empty()));
15591559
}
15601560

1561+
#[test]
1562+
fn test_info_response_serializes_camel_case_and_omits_none() {
1563+
// Guards the JSON wire format (camelCase rename + skip_serializing_if)
1564+
// independently of whether CI env vars were set at compile time.
1565+
let json = serde_json::to_value(InfoResponse {
1566+
pet_version: "1.2.3".to_string(),
1567+
build_id: Some("42".to_string()),
1568+
commit_sha: Some("abc123".to_string()),
1569+
})
1570+
.unwrap();
1571+
assert_eq!(json["petVersion"], "1.2.3");
1572+
assert_eq!(json["buildId"], "42");
1573+
assert_eq!(json["commitSha"], "abc123");
1574+
1575+
let json = serde_json::to_value(InfoResponse {
1576+
pet_version: "1.2.3".to_string(),
1577+
build_id: None,
1578+
commit_sha: None,
1579+
})
1580+
.unwrap();
1581+
assert_eq!(json["petVersion"], "1.2.3");
1582+
assert!(json.get("buildId").is_none());
1583+
assert!(json.get("commitSha").is_none());
1584+
}
1585+
15611586
#[test]
15621587
fn test_parse_refresh_options_rejects_non_empty_array() {
15631588
assert!(parse_refresh_options(json!([{"searchKind": "Conda"}])).is_err());

crates/pet/tests/jsonrpc_server_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ fn assert_single_environment(
111111
}
112112

113113
#[test]
114-
fn info_reports_pet_version_and_optional_build_id() {
114+
fn info_reports_pet_version_and_optional_build_metadata() {
115115
let client = PetJsonRpcClient::spawn().expect("failed to spawn PET server");
116116

117117
let info = client.info().expect("info request failed");

0 commit comments

Comments
 (0)