From 941986b331d7c66ca0b9c4970d5ba0174a027426 Mon Sep 17 00:00:00 2001 From: Henry Guo Date: Mon, 11 May 2026 21:59:24 +0800 Subject: [PATCH] test(e2e): gate protocol runner by requested features (#2912) --- crates/e2e_test/src/common.rs | 77 ++++++++-- crates/e2e_test/src/protocols/README.md | 6 +- crates/e2e_test/src/protocols/test_runner.rs | 149 +++++++++++++++---- 3 files changed, 192 insertions(+), 40 deletions(-) diff --git a/crates/e2e_test/src/common.rs b/crates/e2e_test/src/common.rs index b34f65c55..1c3cef90e 100644 --- a/crates/e2e_test/src/common.rs +++ b/crates/e2e_test/src/common.rs @@ -41,7 +41,9 @@ use walkdir::WalkDir; // Common constants for all E2E tests pub const DEFAULT_ACCESS_KEY: &str = "rustfsadmin"; pub const DEFAULT_SECRET_KEY: &str = "rustfsadmin"; +pub const ENV_RUSTFS_BUILD_FEATURES: &str = "RUSTFS_BUILD_FEATURES"; pub const TEST_BUCKET: &str = "e2e-test-bucket"; +const RUSTFS_FULL_FEATURE: &str = "full"; fn build_test_s3_config(endpoint_url: &str, access_key: &str, secret_key: &str, provider_name: &'static str) -> Config { let credentials = Credentials::new(access_key, secret_key, None, None, provider_name); @@ -83,6 +85,7 @@ pub fn rustfs_binary_path_with_features(requested_features: Option<&str>) -> Pat if let Some(path) = std::env::var_os("CARGO_BIN_EXE_rustfs") { return PathBuf::from(path); } + let requested_features = requested_features.and_then(normalize_rustfs_build_features); let mut binary_path = workspace_root(); binary_path.push("target"); @@ -90,7 +93,7 @@ pub fn rustfs_binary_path_with_features(requested_features: Option<&str>) -> Pat binary_path.push(profile_dir); binary_path.push(format!("rustfs{}", std::env::consts::EXE_SUFFIX)); - let features_match = binary_features_match(&binary_path, requested_features); + let features_match = binary_features_match(&binary_path, requested_features.as_deref()); let source_is_newer = workspace_sources_newer_than_binary(&binary_path); let can_reuse_inside_e2e = running_inside_e2e_test_binary() && requested_features.is_none() && features_match; if binary_path.is_file() && features_match && (!source_is_newer || can_reuse_inside_e2e) { @@ -105,7 +108,7 @@ pub fn rustfs_binary_path_with_features(requested_features: Option<&str>) -> Pat } info!("Building RustFS binary to ensure it's up to date..."); - build_rustfs_binary(requested_features); + build_rustfs_binary(requested_features.as_deref()); info!("Using RustFS binary at {:?}", binary_path); binary_path @@ -134,11 +137,31 @@ fn running_inside_e2e_test_binary() -> bool { std::env::var("CARGO_PKG_NAME").is_ok_and(|value| value == "e2e_test") } -fn requested_rustfs_build_features() -> Option { - std::env::var("RUSTFS_BUILD_FEATURES") +pub fn requested_rustfs_build_features() -> Option { + std::env::var(ENV_RUSTFS_BUILD_FEATURES) .ok() - .map(|value| value.trim().to_string()) - .filter(|value| !value.is_empty()) + .and_then(|value| normalize_rustfs_build_features(&value)) +} + +pub fn normalize_rustfs_build_features(features: &str) -> Option { + let features = features + .split(',') + .map(str::trim) + .filter(|feature| !feature.is_empty()) + .map(str::to_ascii_lowercase) + .collect::>(); + + if features.is_empty() { None } else { Some(features.join(",")) } +} + +pub fn rustfs_build_feature_enabled(requested_features: Option<&str>, required_feature: &str) -> bool { + let Some(requested_features) = requested_features.and_then(normalize_rustfs_build_features) else { + return true; + }; + + requested_features + .split(',') + .any(|feature| feature.eq_ignore_ascii_case(RUSTFS_FULL_FEATURE) || feature.eq_ignore_ascii_case(required_feature)) } fn rustfs_binary_features_stamp_path(binary_path: &Path) -> PathBuf { @@ -147,11 +170,14 @@ fn rustfs_binary_features_stamp_path(binary_path: &Path) -> PathBuf { fn binary_features_match(binary_path: &Path, requested_features: Option<&str>) -> bool { let stamp_path = rustfs_binary_features_stamp_path(binary_path); - let recorded = stdfs::read_to_string(stamp_path).ok().map(|value| value.trim().to_string()); + let recorded = stdfs::read_to_string(stamp_path) + .ok() + .and_then(|value| normalize_rustfs_build_features(&value)); + let requested = requested_features.and_then(normalize_rustfs_build_features); - match requested_features { + match requested.as_deref() { Some(features) => recorded.as_deref() == Some(features), - None => recorded.as_deref().is_none_or(str::is_empty), + None => recorded.is_none(), } } @@ -900,3 +926,36 @@ impl Drop for RustFSTestClusterEnvironment { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn normalizes_rustfs_build_features() { + assert_eq!( + normalize_rustfs_build_features(" SFTP, ftps ,, WebDAV "), + Some("sftp,ftps,webdav".to_string()) + ); + assert_eq!(normalize_rustfs_build_features(" , "), None); + } + + #[test] + fn full_feature_enables_any_required_feature() { + assert!(rustfs_build_feature_enabled(Some("full"), "sftp")); + assert!(rustfs_build_feature_enabled(Some("ftps, full"), "webdav")); + } + + #[test] + fn binary_feature_stamp_matching_uses_normalized_features() { + let binary_path = std::env::temp_dir().join(format!("rustfs-feature-stamp-test-{}", Uuid::new_v4())); + let stamp_path = rustfs_binary_features_stamp_path(&binary_path); + + stdfs::write(&stamp_path, " SFTP, ftps ").expect("write feature stamp"); + assert!(binary_features_match(&binary_path, Some("sftp,ftps"))); + assert!(binary_features_match(&binary_path, Some(" SFTP, FTPS "))); + assert!(!binary_features_match(&binary_path, Some("sftp"))); + + stdfs::remove_file(stamp_path).ok(); + } +} diff --git a/crates/e2e_test/src/protocols/README.md b/crates/e2e_test/src/protocols/README.md index 5ff196d98..0ecd3fad6 100644 --- a/crates/e2e_test/src/protocols/README.md +++ b/crates/e2e_test/src/protocols/README.md @@ -16,9 +16,9 @@ RUSTFS_BUILD_FEATURES=ftps,webdav,sftp cargo test --package e2e_test test_protoc ``` `RUSTFS_BUILD_FEATURES` controls which features the test rustfs binary is -built with. The protocol test runner schedules every entry (FTPS, WebDAV, -SFTP) regardless of the feature set, so the binary must include every -protocol the runner spawns or the corresponding entries will fail. +built with. When this variable is set, the protocol test runner schedules +only entries whose protocol is present in the requested feature list. Leave +it unset to run every protocol entry. `--test-threads=1` is required because every entry spawns a rustfs server on fixed bind ports. diff --git a/crates/e2e_test/src/protocols/test_runner.rs b/crates/e2e_test/src/protocols/test_runner.rs index ff6b70dab..0a578804d 100644 --- a/crates/e2e_test/src/protocols/test_runner.rs +++ b/crates/e2e_test/src/protocols/test_runner.rs @@ -15,6 +15,7 @@ //! Protocol test runner use crate::common::init_logging; +use crate::common::{requested_rustfs_build_features, rustfs_build_feature_enabled}; use crate::protocols::ftps_core::test_ftps_core_operations; use crate::protocols::sftp_compliance::{ test_sftp_compliance_readonly, test_sftp_compliance_standalone, test_sftp_compliance_suite, @@ -59,36 +60,22 @@ pub struct ProtocolTestSuite { #[derive(Debug, Clone)] struct TestDefinition { - name: String, + name: &'static str, + required_feature: &'static str, } impl ProtocolTestSuite { /// Create default test suite pub fn new() -> Self { - let tests = vec![ - TestDefinition { - name: "test_ftps_core_operations".to_string(), - }, - TestDefinition { - name: "test_webdav_core_operations".to_string(), - }, - TestDefinition { - name: "test_sftp_core_operations".to_string(), - }, - TestDefinition { - name: "test_sftp_compliance_suite".to_string(), - }, - TestDefinition { - name: "test_sftp_compliance_readonly".to_string(), - }, - TestDefinition { - name: "test_sftp_idle_timeout_disconnects".to_string(), - }, - TestDefinition { - name: "test_sftp_compliance_standalone".to_string(), - }, - ]; + let requested_features = requested_rustfs_build_features(); + Self::with_requested_features(requested_features.as_deref()) + } + fn with_requested_features(requested_features: Option<&str>) -> Self { + let tests = all_protocol_tests() + .into_iter() + .filter(|test| rustfs_build_feature_enabled(requested_features, test.required_feature)) + .collect(); Self { tests } } @@ -104,7 +91,7 @@ impl ProtocolTestSuite { // Run tests for (i, test_def) in self.tests.iter().enumerate() { - let test_description = match test_def.name.as_str() { + let test_description = match test_def.name { "test_ftps_core_operations" => { info!("=== Starting FTPS Module Test ==="); "FTPS core operations (put, ls, mkdir, rmdir, delete)" @@ -147,11 +134,11 @@ impl ProtocolTestSuite { match result { Ok(_) => { info!("Test passed: {} ({:.2}s)", test_def.name, test_duration.as_secs_f64()); - results.push(TestResult::success(test_def.name.clone())); + results.push(TestResult::success(test_def.name.to_string())); } Err(e) => { error!("Test failed: {} ({:.2}s): {}", test_def.name, test_duration.as_secs_f64(), e); - results.push(TestResult::failure(test_def.name.clone(), e.to_string())); + results.push(TestResult::failure(test_def.name.to_string(), e.to_string())); } } @@ -169,7 +156,7 @@ impl ProtocolTestSuite { /// Run a single test async fn run_single_test(&self, test_def: &TestDefinition) -> Result<(), Box> { - match test_def.name.as_str() { + match test_def.name { "test_ftps_core_operations" => test_ftps_core_operations().await.map_err(|e| e.into()), "test_webdav_core_operations" => test_webdav_core_operations().await.map_err(|e| e.into()), "test_sftp_core_operations" => test_sftp_core_operations().await.map_err(|e| e.into()), @@ -186,6 +173,10 @@ impl ProtocolTestSuite { info!("=== Test Suite Summary ==="); info!("Total duration: {:.2}s", total_duration.as_secs_f64()); info!("Total tests: {}", results.len()); + if results.is_empty() { + info!("No protocol tests scheduled for the requested feature set"); + return; + } let passed = results.iter().filter(|r| r.success).count(); let failed = results.len() - passed; @@ -203,6 +194,39 @@ impl ProtocolTestSuite { } } +fn all_protocol_tests() -> Vec { + vec![ + TestDefinition { + name: "test_ftps_core_operations", + required_feature: "ftps", + }, + TestDefinition { + name: "test_webdav_core_operations", + required_feature: "webdav", + }, + TestDefinition { + name: "test_sftp_core_operations", + required_feature: "sftp", + }, + TestDefinition { + name: "test_sftp_compliance_suite", + required_feature: "sftp", + }, + TestDefinition { + name: "test_sftp_compliance_readonly", + required_feature: "sftp", + }, + TestDefinition { + name: "test_sftp_idle_timeout_disconnects", + required_feature: "sftp", + }, + TestDefinition { + name: "test_sftp_compliance_standalone", + required_feature: "sftp", + }, + ] +} + /// Test suite #[tokio::test] #[serial] @@ -218,3 +242,72 @@ async fn test_protocol_core_suite() -> Result<(), Box Vec<&'static str> { + suite.tests.into_iter().map(|test| test.name).collect() + } + + #[test] + fn schedules_all_protocol_tests_without_feature_filter() { + let names = scheduled_names(ProtocolTestSuite::with_requested_features(None)); + + assert_eq!(names.len(), 7); + assert!(names.contains(&"test_ftps_core_operations")); + assert!(names.contains(&"test_webdav_core_operations")); + assert!(names.contains(&"test_sftp_core_operations")); + assert!(names.contains(&"test_sftp_compliance_standalone")); + } + + #[test] + fn schedules_only_requested_non_sftp_protocols() { + let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("ftps, webdav"))); + + assert_eq!(names, vec!["test_ftps_core_operations", "test_webdav_core_operations"]); + } + + #[test] + fn schedules_all_sftp_entries_for_sftp_feature() { + let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("sftp"))); + + assert_eq!( + names, + vec![ + "test_sftp_core_operations", + "test_sftp_compliance_suite", + "test_sftp_compliance_readonly", + "test_sftp_idle_timeout_disconnects", + "test_sftp_compliance_standalone", + ] + ); + } + + #[test] + fn feature_filter_is_case_insensitive() { + let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("SFTP"))); + + assert_eq!(names.len(), 5); + assert!(names.iter().all(|name| name.contains("sftp"))); + } + + #[test] + fn full_feature_schedules_all_protocol_tests() { + let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("full"))); + + assert_eq!(names.len(), 7); + assert!(names.contains(&"test_ftps_core_operations")); + assert!(names.contains(&"test_webdav_core_operations")); + assert!(names.contains(&"test_sftp_core_operations")); + assert!(names.contains(&"test_sftp_compliance_standalone")); + } + + #[test] + fn schedules_no_tests_when_requested_features_have_no_protocols() { + let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("diagnostics"))); + + assert!(names.is_empty()); + } +}