From 0a246e37366f1cc65976c36b0cd1043a82d8684a Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Wed, 12 Aug 2026 22:37:01 +0800 Subject: [PATCH] test: assert real behavior in three assertion-less tests (#5993) test_format_v1 (ecstore layout::format) only printed its results; the pinned v1 format.json literal never parsed at all because "this": null fails Uuid deserialization, and the Err was silently discarded. Fix the fixture to the real on-disk shape (MinIO and RustFS always write a concrete disk UUID there) and assert a serialize->parse roundtrip identity plus every pinned field of the literal. test_console_cors_configuration discarded all four parse_cors_origins results; parse_cors_origins returns an opaque CorsLayer, so the test now drives real CORS preflight requests through an axum router and asserts the allow-origin outcomes: wildcard answers any origin with *, a configured list echoes listed origins and refuses unlisted ones, empty/unset configurations allow no cross-origin caller. test_heal_channel_processor_new only constructed the processor; it now asserts the response channel accepts a send. Ref rustfs/backlog#1836 (PR1). --- crates/ecstore/src/layout/format.rs | 32 +++++++++--- crates/heal/src/heal/channel.rs | 12 +++-- rustfs/src/admin/console_test.rs | 76 +++++++++++++++++++++++------ 3 files changed, 95 insertions(+), 25 deletions(-) diff --git a/crates/ecstore/src/layout/format.rs b/crates/ecstore/src/layout/format.rs index f0a75abf1..6f6f6de79 100644 --- a/crates/ecstore/src/layout/format.rs +++ b/crates/ecstore/src/layout/format.rs @@ -234,11 +234,17 @@ mod test { #[test] fn test_format_v1() { + // A freshly created format must survive a serialize -> parse roundtrip + // unchanged (identity on every on-disk field). let format = FormatV3::new(1, 4); + let serialized = serde_json::to_string(&format).expect("FormatV3 must serialize to JSON"); + let reparsed = FormatV3::try_from(serialized.as_str()).expect("serialized FormatV3 must parse back"); + assert_eq!(reparsed, format); - let str = serde_json::to_string(&format); - println!("{str:?}"); - + // minio-file-format-compat: this literal pins the on-disk format.json + // shape (erasure version "1", distributionAlgo "CRCMOD"). `this` always + // carries the disk's own UUID in real format.json files; a JSON null + // there was never parseable and never written by MinIO or RustFS. let data = r#" { "version": "1", @@ -246,7 +252,7 @@ mod test { "id": "321b3874-987d-4c15-8fa5-757c956b1243", "xl": { "version": "1", - "this": null, + "this": "8ab9a908-f869-4f1f-8e42-eb067ffa7eb5", "sets": [ [ "8ab9a908-f869-4f1f-8e42-eb067ffa7eb5", @@ -259,9 +265,23 @@ mod test { } }"#; - let p = FormatV3::try_from(data); + let parsed = FormatV3::try_from(data).expect("pinned v1 format.json literal must keep parsing"); - println!("{p:?}"); + assert_eq!(parsed.version, FormatMetaVersion::V1); + assert_eq!(parsed.format, FormatBackend::Erasure); + assert_eq!( + parsed.id, + Uuid::parse_str("321b3874-987d-4c15-8fa5-757c956b1243").expect("literal id is a valid UUID") + ); + assert_eq!(parsed.erasure.version, FormatErasureVersion::V1); + assert_eq!( + parsed.erasure.this, + Uuid::parse_str("8ab9a908-f869-4f1f-8e42-eb067ffa7eb5").expect("literal this is a valid UUID") + ); + assert_eq!(parsed.erasure.sets.len(), 1); + assert_eq!(parsed.erasure.sets[0].len(), 4); + assert_eq!(parsed.erasure.sets[0][0], parsed.erasure.this); + assert_eq!(parsed.erasure.distribution_algo, DistributionAlgoVersion::V1); } #[test] diff --git a/crates/heal/src/heal/channel.rs b/crates/heal/src/heal/channel.rs index 9603b81c0..d2be2545d 100644 --- a/crates/heal/src/heal/channel.rs +++ b/crates/heal/src/heal/channel.rs @@ -785,9 +785,15 @@ mod tests { let heal_manager = create_test_heal_manager(); let processor = HealChannelProcessor::new(heal_manager); - // Verify processor is created successfully - let _sender = processor.get_response_sender(); - // If we can get the sender, processor was created correctly + let sender = processor.get_response_sender(); + sender + .send(HealChannelResponse { + request_id: "request-id".to_string(), + success: true, + data: None, + error: None, + }) + .expect("a freshly constructed processor must accept responses on its channel"); } #[test] diff --git a/rustfs/src/admin/console_test.rs b/rustfs/src/admin/console_test.rs index 6fa36cc13..0d600bebc 100644 --- a/rustfs/src/admin/console_test.rs +++ b/rustfs/src/admin/console_test.rs @@ -17,29 +17,73 @@ mod tests { use crate::config::Opt; use serial_test::serial; + /// Sends a CORS preflight request through a router wrapped with the given + /// layer and returns the `access-control-allow-origin` header, if any. + async fn preflight_allow_origin(cors: tower_http::cors::CorsLayer, origin: &str) -> Option { + use axum::{Router, body::Body, routing::get}; + use tower::ServiceExt; + + let app = Router::new().route("/", get(|| async { "ok" })).layer(cors); + let response = app + .oneshot( + http::Request::builder() + .method(http::Method::OPTIONS) + .uri("/") + .header(http::header::ORIGIN, origin) + .header(http::header::ACCESS_CONTROL_REQUEST_METHOD, "GET") + .body(Body::empty()) + .expect("preflight request must build"), + ) + .await + .expect("preflight request must not fail"); + + response + .headers() + .get(http::header::ACCESS_CONTROL_ALLOW_ORIGIN) + .map(|value| value.to_str().expect("allow-origin header must be valid UTF-8").to_string()) + } + #[tokio::test] #[serial] async fn test_console_cors_configuration() { - // Test CORS configuration parsing use crate::admin::console::parse_cors_origins; - // Test wildcard origin - let cors_wildcard = Some("*".to_string()); - let _layer1 = parse_cors_origins(cors_wildcard.as_ref()); - // Should create a layer without error - // Test specific origins - let cors_specific = Some("http://localhost:3000,https://admin.example.com".to_string()); - let _layer2 = parse_cors_origins(cors_specific.as_ref()); - // Should create a layer without error + // Wildcard configuration must allow any origin. + let wildcard = parse_cors_origins(Some(&"*".to_string())); + assert_eq!( + preflight_allow_origin(wildcard, "http://anywhere.example").await.as_deref(), + Some("*"), + "wildcard configuration must answer preflight with a permissive allow-origin" + ); - // Test empty origin - let cors_empty = Some("".to_string()); - let _layer3 = parse_cors_origins(cors_empty.as_ref()); - // Should create a layer without error (falls back to permissive) + // An explicit list must echo listed origins and refuse unlisted ones. + let listed = parse_cors_origins(Some(&"http://localhost:3000,https://admin.example.com".to_string())); + assert_eq!( + preflight_allow_origin(listed, "http://localhost:3000").await.as_deref(), + Some("http://localhost:3000"), + "a listed origin must be echoed back on preflight" + ); + let listed = parse_cors_origins(Some(&"http://localhost:3000,https://admin.example.com".to_string())); + assert_eq!( + preflight_allow_origin(listed, "https://other.example").await, + None, + "an unlisted origin must not receive an allow-origin header" + ); - // Test no origin - let _layer4 = parse_cors_origins(None); - // Should create a layer without error (uses default) + // Empty and unset configurations fall back to same-origin only: + // no cross-origin caller may be allowed. + let empty = parse_cors_origins(Some(&"".to_string())); + assert_eq!( + preflight_allow_origin(empty, "http://localhost:3000").await, + None, + "empty configuration must not allow any cross-origin caller" + ); + let unset = parse_cors_origins(None); + assert_eq!( + preflight_allow_origin(unset, "http://localhost:3000").await, + None, + "unset configuration must not allow any cross-origin caller" + ); } #[tokio::test]